Fix WIC encoder to support kJPEG_Type
authormsarett <msarett@google.com>
Tue, 16 Aug 2016 01:52:17 +0000 (18:52 -0700)
committerCommit bot <commit-bot@chromium.org>
Tue, 16 Aug 2016 01:52:17 +0000 (18:52 -0700)
(1) Add support for kJPEG to WIC
(2) Add encoding test.
(3) Turn on WIC jpeg encoder on Windows and CG jpeg
    encoder on Mac.

A follow-up may make Skia's encoders the default on all
platforms.  But, in order to do that, I think we need
to write better encoding unit tests for CG and WIC.

BUG=skia:3969
BUG=skia:5632
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2245453002

Committed: https://skia.googlesource.com/skia/+/b3a7ef1fc0adc24859d2498aee54d3ec2cbcac3a
Review-Url: https://codereview.chromium.org/2245453002

gm/encode.cpp [new file with mode: 0644]
src/images/SkForceLinking.cpp
src/ports/SkImageEncoder_WIC.cpp

diff --git a/gm/encode.cpp b/gm/encode.cpp
new file mode 100644 (file)
index 0000000..0cd562a
--- /dev/null
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2011 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+#include "gm.h"
+#include "SkCanvas.h"
+#include "SkData.h"
+#include "SkImageEncoder.h"
+#include "Resources.h"
+
+namespace skiagm {
+
+class EncodeGM : public GM {
+public:
+    EncodeGM() {}
+
+protected:
+    SkString onShortName() override {
+        return SkString("encode");
+    }
+
+    SkISize onISize() override {
+        return SkISize::Make(1024, 600);
+    }
+
+    void onDraw(SkCanvas* canvas) override {
+        SkBitmap orig;
+        GetResourceAsBitmap("mandrill_512_q075.jpg", &orig);
+        sk_sp<SkData> pngData(SkImageEncoder::EncodeData(orig, SkImageEncoder::kPNG_Type, 100));
+        sk_sp<SkData> jpegData(SkImageEncoder::EncodeData(orig, SkImageEncoder::kJPEG_Type, 100));
+
+        sk_sp<SkImage> pngImage = SkImage::MakeFromEncoded(pngData);
+        sk_sp<SkImage> jpegImage = SkImage::MakeFromEncoded(jpegData);
+        canvas->drawImage(pngImage.get(), 0.0f, 0.0f);
+        canvas->drawImage(jpegImage.get(), 512.0f, 0.0f);
+
+        const char text[] = "Images should look identical.";
+        canvas->drawText(text, sizeof(text) - 1, 450.0f, 550.0f, SkPaint());
+    }
+
+private:
+    typedef GM INHERITED;
+};
+
+DEF_GM( return new EncodeGM; )
+}
index e97106fdf838359cc16aa8df67c2118707df1ed0..2afe7192a9e3a05661973deb18939881052bae99 100644 (file)
@@ -14,7 +14,8 @@
 int SkForceLinking(bool doNotPassTrue) {
     if (doNotPassTrue) {
         SkASSERT(false);
-#if defined(SK_HAS_JPEG_LIBRARY)
+#if !defined(SK_BUILD_FOR_MAC) && !defined(SK_BUILD_FOR_WIN) && !defined(SK_BUILD_FOR_IOS) && \
+    defined(SK_HAS_JPEG_LIBRARY)
         CreateJPEGImageEncoder();
 #endif
 #if defined(SK_HAS_WEBP_LIBRARY)
index 5523ea2e11e267e13ad049e48752043a5bf5c9e8..9be95727cad2b0cd16d82da44ae46def4cbc6092 100644 (file)
@@ -83,35 +83,49 @@ bool SkImageEncoder_WIC::onEncode(SkWStream* stream
             return false;
     }
 
-    //Convert to 8888 if needed.
-    const SkBitmap* bitmap;
-    SkBitmap bitmapCopy;
-    if (kN32_SkColorType == bitmapOrig.colorType() && bitmapOrig.isOpaque()) {
-        bitmap = &bitmapOrig;
-    } else {
-        if (!bitmapOrig.copyTo(&bitmapCopy, kN32_SkColorType)) {
-            return false;
-        }
-        bitmap = &bitmapCopy;
+    // First convert to BGRA if necessary.
+    SkBitmap bitmap;
+    if (!bitmapOrig.copyTo(&bitmap, kBGRA_8888_SkColorType)) {
+        return false;
     }
 
-    // We cannot use PBGRA so we need to unpremultiply ourselves
-    if (!bitmap->isOpaque()) {
-        SkAutoLockPixels alp(*bitmap);
-
-        uint8_t* pixels = reinterpret_cast<uint8_t*>(bitmap->getPixels());
-        for (int y = 0; y < bitmap->height(); ++y) {
-            for (int x = 0; x < bitmap->width(); ++x) {
-                uint8_t* bytes = pixels + y * bitmap->rowBytes() + x * bitmap->bytesPerPixel();
-
+    // WIC expects unpremultiplied pixels.  Unpremultiply if necessary.
+    if (kPremul_SkAlphaType == bitmap.alphaType()) {
+        uint8_t* pixels = reinterpret_cast<uint8_t*>(bitmap.getPixels());
+        for (int y = 0; y < bitmap.height(); ++y) {
+            for (int x = 0; x < bitmap.width(); ++x) {
+                uint8_t* bytes = pixels + y * bitmap.rowBytes() + x * bitmap.bytesPerPixel();
                 SkPMColor* src = reinterpret_cast<SkPMColor*>(bytes);
                 SkColor* dst = reinterpret_cast<SkColor*>(bytes);
-
                 *dst = SkUnPreMultiply::PMColorToColor(*src);
             }
         }
     }
 
+    // Finally, if we are performing a jpeg encode, we must convert to BGR.
+    void* pixels = bitmap.getPixels();
+    size_t rowBytes = bitmap.rowBytes();
+    SkAutoMalloc pixelStorage;
+    WICPixelFormatGUID formatDesired = GUID_WICPixelFormat32bppBGRA;
+    if (kJPEG_Type == fType) {
+        formatDesired = GUID_WICPixelFormat24bppBGR;
+        rowBytes = SkAlign4(bitmap.width() * 3);
+        pixelStorage.reset(rowBytes * bitmap.height());
+        for (int y = 0; y < bitmap.height(); y++) {
+            uint8_t* dstRow = SkTAddOffset<uint8_t>(pixelStorage.get(), y * rowBytes);
+            for (int x = 0; x < bitmap.width(); x++) {
+                uint32_t bgra = *bitmap.getAddr32(x, y);
+                dstRow[0] = (uint8_t) (bgra >>  0);
+                dstRow[1] = (uint8_t) (bgra >>  8);
+                dstRow[2] = (uint8_t) (bgra >> 16);
+                dstRow += 3;
+            }
+        }
+
+        pixels = pixelStorage.get();
+    }
+
+
     //Initialize COM.
     SkAutoCoInitialize scopedCo;
     if (!scopedCo.succeeded()) {
@@ -176,15 +190,14 @@ bool SkImageEncoder_WIC::onEncode(SkWStream* stream
     }
 
     //Set the size of the frame.
-    const UINT width = bitmap->width();
-    const UINT height = bitmap->height();
+    const UINT width = bitmap.width();
+    const UINT height = bitmap.height();
     if (SUCCEEDED(hr)) {
         hr = piBitmapFrameEncode->SetSize(width, height);
     }
 
     //Set the pixel format of the frame.  If native encoded format cannot match BGRA,
     //it will choose the closest pixel format that it supports.
-    const WICPixelFormatGUID formatDesired = GUID_WICPixelFormat32bppBGRA;
     WICPixelFormatGUID formatGUID = formatDesired;
     if (SUCCEEDED(hr)) {
         hr = piBitmapFrameEncode->SetPixelFormat(&formatGUID);
@@ -196,13 +209,10 @@ bool SkImageEncoder_WIC::onEncode(SkWStream* stream
 
     //Write the pixels into the frame.
     if (SUCCEEDED(hr)) {
-        SkAutoLockPixels alp(*bitmap);
-        const UINT stride = (UINT) bitmap->rowBytes();
-        hr = piBitmapFrameEncode->WritePixels(
-            height
-            , stride
-            , stride * height
-            , reinterpret_cast<BYTE*>(bitmap->getPixels()));
+        hr = piBitmapFrameEncode->WritePixels(height,
+                                              (UINT) rowBytes,
+                                              (UINT) rowBytes * height,
+                                              reinterpret_cast<BYTE*>(pixels));
     }
 
     if (SUCCEEDED(hr)) {
@@ -223,6 +233,7 @@ static SkImageEncoder* sk_imageencoder_wic_factory(SkImageEncoder::Type t) {
         case SkImageEncoder::kBMP_Type:
         case SkImageEncoder::kICO_Type:
         case SkImageEncoder::kPNG_Type:
+        case SkImageEncoder::kJPEG_Type:
             break;
         default:
             return nullptr;