Revert "Add support for row-by-row jpeg encoding"
authorLeon Scroggins <scroggo@google.com>
Tue, 2 May 2017 17:08:28 +0000 (17:08 +0000)
committerSkia Commit-Bot <skia-commit-bot@chromium.org>
Tue, 2 May 2017 17:08:35 +0000 (17:08 +0000)
This reverts commit 9b848d5749c5e34b56f927a3a3374c8ebafbd9db.

Reason for revert: ASAN reports leaked memory [1]. Google3 reports a "delete size mismatch" [2], which I suspect is the same issue.

[1] https://chromium-swarm.appspot.com/task?id=35e2c9fa9eac6310&refresh=10&show_raw=1
[2] https://test.corp.google.com/ui#cl=154838904&flags=CAMQBQ==&id=OCL:154838904:BASE:154839043:1493741642370:9c96115f&t=//chrome/skia/dm_wrapper:dm_wrapper

Original change's description:
> Add support for row-by-row jpeg encoding
>
> Bug: 713862
> Change-Id: I787b7c49662a00b89ae0ef35845dfbd6be3e6fb1
> Reviewed-on: https://skia-review.googlesource.com/14641
> Commit-Queue: Matt Sarett <msarett@google.com>
> Reviewed-by: Leon Scroggins <scroggo@google.com>
>

TBR=msarett@google.com,scroggo@google.com,reed@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: Ic5a8d67e0d4a7733662586055ceff086a2ab335d
Reviewed-on: https://skia-review.googlesource.com/15140
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

12 files changed:
gm/encode-platform.cpp
gm/encode-srgb.cpp
gn/tests.gni
src/core/SkImageInfoPriv.h
src/gpu/SkGr.cpp
src/images/SkImageEncoder.cpp
src/images/SkImageEncoderPriv.h
src/images/SkJPEGImageEncoder.cpp
src/images/SkJpegEncoder.h [deleted file]
tests/CodecTest.cpp
tests/EncodeTest.cpp [deleted file]
tests/ReadPixelsTest.cpp

index f53ebc0..f08da73 100644 (file)
@@ -11,7 +11,6 @@
 #include "SkCanvas.h"
 #include "SkData.h"
 #include "SkImageEncoderPriv.h"
-#include "SkJpegEncoder.h"
 #include "SkUnPreMultiply.h"
 
 namespace skiagm {
@@ -71,10 +70,8 @@ static sk_sp<SkData> encode_data(SkEncodedImageFormat type, const SkBitmap& bitm
             case SkEncodedImageFormat::kPNG:
                 return SkEncodeImageAsPNG(&buf, src, SkEncodeOptions()) ? buf.detachAsData()
                                                                         : nullptr;
-            case SkEncodedImageFormat::kJPEG: {
-                bool success = SkJpegEncoder::Encode(&buf, src, SkJpegEncoder::Options());
-                return success ? buf.detachAsData() : nullptr;
-            }
+            case SkEncodedImageFormat::kJPEG:
+                return SkEncodeImageAsJPEG(&buf, src, 100) ? buf.detachAsData() : nullptr;
             case SkEncodedImageFormat::kWEBP:
                 return SkEncodeImageAsWEBP(&buf, src, 100) ? buf.detachAsData() : nullptr;
             default:
index fef1332..3015baa 100644 (file)
@@ -13,7 +13,6 @@
 #include "SkColorSpace_Base.h"
 #include "SkData.h"
 #include "SkImageEncoderPriv.h"
-#include "SkJpegEncoder.h"
 #include "SkPM4f.h"
 #include "SkSRGB.h"
 
@@ -129,7 +128,7 @@ static sk_sp<SkData> encode_data(const SkBitmap& bitmap, SkEncodedImageFormat fo
             SkAssertResult(SkEncodeImageAsWEBP(&buf, src, options));
             break;
         case SkEncodedImageFormat::kJPEG:
-            SkAssertResult(SkJpegEncoder::Encode(&buf, src, SkJpegEncoder::Options()));
+            SkAssertResult(SkEncodeImageAsJPEG(&buf, src, options));
             break;
         default:
             break;
index be4389b..ffd8b41 100644 (file)
@@ -63,7 +63,6 @@ tests_sources = [
   "$_tests/DynamicHashTest.cpp",
   "$_tests/EGLImageTest.cpp",
   "$_tests/EmptyPathTest.cpp",
-  "$_tests/EncodeTest.cpp",
   "$_tests/ExifTest.cpp",
   "$_tests/F16StagesTest.cpp",
   "$_tests/FillPathTest.cpp",
index a0c1b34..855e506 100644 (file)
 #include "SkImageInfo.h"
 
 /**
- *  This contains shared checks on SkImageInfo.  Depending on the desired color space behavior,
- *  the caller should choose one of the two versions below.
+ *  Returns true if |info| contains a valid combination of width, height, colorType, alphaType,
+ *  colorSpace.  Returns false otherwise.
  */
-static inline bool SkImageInfoIsValidCommon(const SkImageInfo& info) {
+static inline bool SkImageInfoIsValid(const SkImageInfo& info) {
     if (info.width() <= 0 || info.height() <= 0) {
         return false;
     }
@@ -38,35 +38,6 @@ static inline bool SkImageInfoIsValidCommon(const SkImageInfo& info) {
         return false;
     }
 
-    return true;
-}
-
-/**
- *  Returns true if |info| contains a valid combination of width, height, colorType, alphaType,
- *  colorSpace.  Allows numerical color spaces.  Returns false otherwise.
- */
-static inline bool SkImageInfoIsValidAllowNumericalCS(const SkImageInfo& info) {
-    if (!SkImageInfoIsValidCommon(info)) {
-        return false;
-    }
-
-    SkColorSpaceTransferFn fn;
-    if (info.colorSpace() && !info.colorSpace()->isNumericalTransferFn(&fn)) {
-        return false;
-    }
-
-    return true;
-}
-
-/**
- *  Returns true if |info| contains a valid combination of width, height, colorType, alphaType,
- *  colorSpace.  Only supports rendering color spaces.  Returns false otherwise.
- */
-static inline bool SkImageInfoIsValidRenderingCS(const SkImageInfo& info) {
-    if (!SkImageInfoIsValidCommon(info)) {
-        return false;
-    }
-
     if (info.colorSpace() &&
        (!info.colorSpace()->gammaCloseToSRGB() && !info.colorSpace()->gammaIsLinear())) {
         return false;
@@ -94,7 +65,7 @@ static inline bool SkImageInfoIsValidRenderingCS(const SkImageInfo& info) {
  *      conversion is not well-defined.
  */
 static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkImageInfo& src) {
-    if (!SkImageInfoIsValidRenderingCS(dst) || !SkImageInfoIsValidRenderingCS(src)) {
+    if (!SkImageInfoIsValid(dst) || !SkImageInfoIsValid(src)) {
         return false;
     }
 
index 895fdec..5401a10 100644 (file)
@@ -176,7 +176,7 @@ sk_sp<GrTextureProxy> GrUploadPixmapToTextureProxyNoCheck(GrResourceProvider* re
 sk_sp<GrTextureProxy> GrUploadPixmapToTextureProxy(GrResourceProvider* resourceProvider,
                                                    const SkPixmap& pixmap,
                                                    SkBudgeted budgeted) {
-    if (!SkImageInfoIsValidRenderingCS(pixmap.info())) {
+    if (!SkImageInfoIsValid(pixmap.info())) {
         return nullptr;
     }
 
@@ -205,7 +205,7 @@ sk_sp<GrTextureProxy> GrGenerateMipMapsAndUploadToTextureProxy(GrContext* ctx,
         ? SkDestinationSurfaceColorMode::kGammaAndColorSpaceAware
         : SkDestinationSurfaceColorMode::kLegacy;
 
-    if (!SkImageInfoIsValidRenderingCS(bitmap.info())) {
+    if (!SkImageInfoIsValid(bitmap.info())) {
         return nullptr;
     }
 
@@ -252,7 +252,7 @@ sk_sp<GrTextureProxy> GrUploadMipMapToTextureProxy(GrContext* ctx, const SkImage
                                                    const GrMipLevel* texels,
                                                    int mipLevelCount,
                                                    SkDestinationSurfaceColorMode colorMode) {
-    if (!SkImageInfoIsValidRenderingCS(info)) {
+    if (!SkImageInfoIsValid(info)) {
         return nullptr;
     }
 
index 23f46e4..fecadbf 100644 (file)
@@ -6,7 +6,6 @@
  */
 
 #include "SkImageEncoderPriv.h"
-#include "SkJpegEncoder.h"
 
 bool SkEncodeImage(SkWStream* dst, const SkPixmap& src,
                    SkEncodedImageFormat format, int quality) {
@@ -17,11 +16,8 @@ bool SkEncodeImage(SkWStream* dst, const SkPixmap& src,
         return SkEncodeImageWithWIC(dst, src, format, quality);
     #else
         switch(format) {
-            case SkEncodedImageFormat::kJPEG: {
-                SkJpegEncoder::Options opts;
-                opts.fQuality = quality;
-                return SkJpegEncoder::Encode(dst, src, opts);
-            }
+            case SkEncodedImageFormat::kJPEG:
+                return SkEncodeImageAsJPEG(dst, src, quality);
             case SkEncodedImageFormat::kPNG:
                 return SkEncodeImageAsPNG(dst, src, SkEncodeOptions());
             case SkEncodedImageFormat::kWEBP:
index 5e57cd1..540d930 100644 (file)
@@ -14,6 +14,13 @@ struct SkEncodeOptions {
     SkTransferFunctionBehavior fUnpremulBehavior = SkTransferFunctionBehavior::kIgnore;
 };
 
+#ifdef SK_HAS_JPEG_LIBRARY
+    bool SkEncodeImageAsJPEG(SkWStream*, const SkPixmap&, const SkEncodeOptions&);
+    bool SkEncodeImageAsJPEG(SkWStream*, const SkPixmap&, int quality);
+#else
+    #define SkEncodeImageAsJPEG(...) false
+#endif
+
 #ifdef SK_HAS_PNG_LIBRARY
     bool SkEncodeImageAsPNG(SkWStream*, const SkPixmap&, const SkEncodeOptions&);
 #else
index b875898..764e9c4 100644 (file)
@@ -11,8 +11,6 @@
 
 #include "SkColorPriv.h"
 #include "SkImageEncoderFns.h"
-#include "SkImageInfoPriv.h"
-#include "SkJpegEncoder.h"
 #include "SkJPEGWriteUtility.h"
 #include "SkStream.h"
 #include "SkTemplates.h"
@@ -24,144 +22,122 @@ extern "C" {
     #include "jerror.h"
 }
 
-// This warning triggers false postives way too often in here.
-#if defined(__GNUC__) && !defined(__clang__)
-    #pragma GCC diagnostic ignored "-Wclobbered"
-#endif
-
-class SkJpegEncoderMgr : SkNoncopyable {
-public:
-
-    /*
-     * Create the decode manager
-     * Does not take ownership of stream
-     */
-    static std::unique_ptr<SkJpegEncoderMgr> Make(SkWStream* stream) {
-        return std::unique_ptr<SkJpegEncoderMgr>(new SkJpegEncoderMgr(stream));
-    }
-
-    bool setParams(const SkImageInfo& srcInfo);
-
-    jpeg_compress_struct* cinfo() { return &fCInfo; }
-
-    jmp_buf& jmpBuf() { return fErrMgr.fJmpBuf; }
-
-    transform_scanline_proc proc() const { return fProc; }
-
-    ~SkJpegEncoderMgr() {
-        jpeg_destroy_compress(&fCInfo);
-    }
-
-private:
-
-    SkJpegEncoderMgr(SkWStream* stream)
-        : fDstMgr(stream)
-        , fProc(nullptr)
-    {
-        fCInfo.err = jpeg_std_error(&fErrMgr);
-        fErrMgr.error_exit = skjpeg_error_exit;
-        jpeg_create_compress(&fCInfo);
-        fCInfo.dest = &fDstMgr;
-    }
-
-    jpeg_compress_struct    fCInfo;
-    skjpeg_error_mgr        fErrMgr;
-    skjpeg_destination_mgr  fDstMgr;
-    transform_scanline_proc fProc;
-};
-
-bool SkJpegEncoderMgr::setParams(const SkImageInfo& srcInfo) {
-    J_COLOR_SPACE jpegColorType = JCS_EXT_RGBA;
-    int numComponents = 0;
-    switch (srcInfo.colorType()) {
+/**
+ *  Returns true if |info| is supported by the jpeg encoder and false otherwise.
+ *  |jpegColorType| will be set to the proper libjpeg-turbo type for input to the library.
+ *  |numComponents| will be set to the number of components in the |jpegColorType|.
+ *  |proc|          will be set if we need to pre-convert the input before passing to
+ *                  libjpeg-turbo.  Otherwise will be set to nullptr.
+ */
+// TODO (skbug.com/1501):
+// Should we fail on non-opaque encodes?
+// Or should we change alpha behavior (ex: unpremultiply when the input is premul)?
+// Or is ignoring the alpha type and alpha channel ok here?
+static bool set_encode_config(J_COLOR_SPACE* jpegColorType, int* numComponents,
+                              transform_scanline_proc* proc, const SkImageInfo& info) {
+    *proc = nullptr;
+    switch (info.colorType()) {
         case kRGBA_8888_SkColorType:
-            jpegColorType = JCS_EXT_RGBA;
-            numComponents = 4;
-            break;
+            *jpegColorType = JCS_EXT_RGBA;
+            *numComponents = 4;
+            return true;
         case kBGRA_8888_SkColorType:
-            jpegColorType = JCS_EXT_BGRA;
-            numComponents = 4;
-            break;
+            *jpegColorType = JCS_EXT_BGRA;
+            *numComponents = 4;
+            return true;
         case kRGB_565_SkColorType:
-            fProc = transform_scanline_565;
-            jpegColorType = JCS_RGB;
-            numComponents = 3;
-            break;
+            *proc = transform_scanline_565;
+            *jpegColorType = JCS_RGB;
+            *numComponents = 3;
+            return true;
         case kARGB_4444_SkColorType:
-            fProc = transform_scanline_444;
-            jpegColorType = JCS_RGB;
-            numComponents = 3;
-            break;
+            *proc = transform_scanline_444;
+            *jpegColorType = JCS_RGB;
+            *numComponents = 3;
+            return true;
         case kIndex_8_SkColorType:
-            fProc = transform_scanline_index8_opaque;
-            jpegColorType = JCS_RGB;
-            numComponents = 3;
-            break;
+            *proc = transform_scanline_index8_opaque;
+            *jpegColorType = JCS_RGB;
+            *numComponents = 3;
+            return true;
         case kGray_8_SkColorType:
-            SkASSERT(srcInfo.isOpaque());
-            jpegColorType = JCS_GRAYSCALE;
-            numComponents = 1;
-            break;
+            SkASSERT(info.isOpaque());
+            *jpegColorType = JCS_GRAYSCALE;
+            *numComponents = 1;
+            return true;
         case kRGBA_F16_SkColorType:
-            if (!srcInfo.colorSpace() || !srcInfo.colorSpace()->gammaIsLinear()) {
+            if (!info.colorSpace() || !info.colorSpace()->gammaIsLinear()) {
                 return false;
             }
 
-            fProc = transform_scanline_F16_to_8888;
-            jpegColorType = JCS_EXT_RGBA;
-            numComponents = 4;
-            break;
+            *proc = transform_scanline_F16_to_8888;
+            *jpegColorType = JCS_EXT_RGBA;
+            *numComponents = 4;
+            return true;
         default:
             return false;
     }
 
-    fCInfo.image_width = srcInfo.width();
-    fCInfo.image_height = srcInfo.height();
-    fCInfo.in_color_space = jpegColorType;
-    fCInfo.input_components = numComponents;
-    jpeg_set_defaults(&fCInfo);
 
-    // Tells libjpeg-turbo to compute optimal Huffman coding tables
-    // for the image.  This improves compression at the cost of
-    // slower encode performance.
-    fCInfo.optimize_coding = TRUE;
-    return true;
 }
 
-class SkJpegEncoder_Base : public SkJpegEncoder {
-public:
-    SkJpegEncoder_Base(std::unique_ptr<SkJpegEncoderMgr> encoderMgr, const SkPixmap& src);
-
-    bool onEncodeRows(int numRows);
+bool SkEncodeImageAsJPEG(SkWStream* stream, const SkPixmap& pixmap, const SkEncodeOptions& opts) {
+    if (SkTransferFunctionBehavior::kRespect == opts.fUnpremulBehavior) {
+        // Respecting the transfer function requries a color space.  It's not actually critical
+        // in the jpeg case (since jpegs are opaque), but Skia color correct behavior generally
+        // requires pixels to be tagged with color spaces.
+        if (!pixmap.colorSpace() || (!pixmap.colorSpace()->gammaCloseToSRGB() &&
+                                     !pixmap.colorSpace()->gammaIsLinear())) {
+            return false;
+        }
+    }
 
-private:
-    std::unique_ptr<SkJpegEncoderMgr> fEncoderMgr;
-    SkPixmap                          fSrc;
-    int                               fCurrRow;
-    SkAutoTMalloc<uint8_t>            fStorage;
-};
+    return SkEncodeImageAsJPEG(stream, pixmap, 100);
+}
 
-std::unique_ptr<SkJpegEncoder> SkJpegEncoder::Make(SkWStream* dst, const SkPixmap& src,
-                                                   const Options& options) {
-    if (!SkImageInfoIsValidAllowNumericalCS(src.info()) || !src.addr() ||
-            src.rowBytes() < src.info().minRowBytes()) {
-        return nullptr;
+bool SkEncodeImageAsJPEG(SkWStream* stream, const SkPixmap& pixmap, int quality) {
+    if (!pixmap.addr()) {
+        return false;
     }
+    jpeg_compress_struct    cinfo;
+    skjpeg_error_mgr        sk_err;
+    skjpeg_destination_mgr  sk_wstream(stream);
 
-    std::unique_ptr<SkJpegEncoderMgr> encoderMgr = SkJpegEncoderMgr::Make(dst);
-    if (setjmp(encoderMgr->jmpBuf())) {
-        return nullptr;
+    // Declare before calling setjmp.
+    SkAutoTMalloc<uint8_t>  storage;
+
+    cinfo.err = jpeg_std_error(&sk_err);
+    sk_err.error_exit = skjpeg_error_exit;
+    if (setjmp(sk_err.fJmpBuf)) {
+        return false;
     }
 
-    if (!encoderMgr->setParams(src.info())) {
-        return nullptr;
+    J_COLOR_SPACE jpegColorSpace;
+    int numComponents;
+    transform_scanline_proc proc;
+    if (!set_encode_config(&jpegColorSpace, &numComponents, &proc, pixmap.info())) {
+        return false;
     }
 
-    jpeg_set_quality(encoderMgr->cinfo(), options.fQuality, TRUE);
-    jpeg_start_compress(encoderMgr->cinfo(), TRUE);
+    jpeg_create_compress(&cinfo);
+    cinfo.dest = &sk_wstream;
+    cinfo.image_width = pixmap.width();
+    cinfo.image_height = pixmap.height();
+    cinfo.input_components = numComponents;
+    cinfo.in_color_space = jpegColorSpace;
+
+    jpeg_set_defaults(&cinfo);
 
-    if (src.colorSpace()) {
-        sk_sp<SkData> icc = icc_from_color_space(*src.colorSpace());
+    // Tells libjpeg-turbo to compute optimal Huffman coding tables
+    // for the image.  This improves compression at the cost of
+    // slower encode performance.
+    cinfo.optimize_coding = TRUE;
+    jpeg_set_quality(&cinfo, quality, TRUE /* limit to baseline-JPEG values */);
+
+    jpeg_start_compress(&cinfo, TRUE);
+
+    if (pixmap.colorSpace()) {
+        sk_sp<SkData> icc = icc_from_color_space(*pixmap.colorSpace());
         if (icc) {
             // Create a contiguous block of memory with the icc signature followed by the profile.
             sk_sp<SkData> markerData =
@@ -173,68 +149,30 @@ std::unique_ptr<SkJpegEncoder> SkJpegEncoder::Make(SkWStream* dst, const SkPixma
             *ptr++ = 1; // Out of one total markers.
             memcpy(ptr, icc->data(), icc->size());
 
-            jpeg_write_marker(encoderMgr->cinfo(), kICCMarker, markerData->bytes(),
-                              markerData->size());
+            jpeg_write_marker(&cinfo, kICCMarker, markerData->bytes(), markerData->size());
         }
     }
 
-    return std::unique_ptr<SkJpegEncoder>(new SkJpegEncoder_Base(std::move(encoderMgr), src));
-}
-
-
-SkJpegEncoder_Base::SkJpegEncoder_Base(std::unique_ptr<SkJpegEncoderMgr> encoderMgr,
-                                       const SkPixmap& src)
-    : fEncoderMgr(std::move(encoderMgr))
-    , fSrc(src)
-    , fCurrRow(0)
-    , fStorage(fEncoderMgr->proc() ? fEncoderMgr->cinfo()->input_components*src.width() : 0)
-{}
-
-bool SkJpegEncoder::encodeRows(int numRows) {
-    return ((SkJpegEncoder_Base*) this)->onEncodeRows(numRows);
-}
-
-bool SkJpegEncoder_Base::onEncodeRows(int numRows) {
-    SkASSERT(numRows > 0 && fCurrRow < fSrc.height());
-    if (numRows <= 0 || fCurrRow >= fSrc.height()) {
-        return false;
-    }
-
-    if (fCurrRow + numRows > fSrc.height()) {
-        numRows = fSrc.height() - fCurrRow;
-    }
-
-    if (setjmp(fEncoderMgr->jmpBuf())) {
-        // Short circuit any future calls after failing.
-        fCurrRow = fSrc.height();
-        return false;
+    if (proc) {
+        storage.reset(numComponents * pixmap.width());
     }
 
-    const void* srcRow = fSrc.addr(0, fCurrRow);
-    const SkPMColor* colors = fSrc.ctable() ? fSrc.ctable()->readColors() : nullptr;
-    for (int i = 0; i < numRows; i++) {
+    const void* srcRow = pixmap.addr();
+    const SkPMColor* colors = pixmap.ctable() ? pixmap.ctable()->readColors() : nullptr;
+    while (cinfo.next_scanline < cinfo.image_height) {
         JSAMPLE* jpegSrcRow = (JSAMPLE*) srcRow;
-        if (fEncoderMgr->proc()) {
-            fEncoderMgr->proc()((char*)fStorage.get(), (const char*)srcRow, fSrc.width(),
-                                fEncoderMgr->cinfo()->input_components, colors);
-            jpegSrcRow = fStorage.get();
+        if (proc) {
+            proc((char*)storage.get(), (const char*)srcRow, pixmap.width(), numComponents, colors);
+            jpegSrcRow = storage.get();
         }
 
-        jpeg_write_scanlines(fEncoderMgr->cinfo(), &jpegSrcRow, 1);
-        srcRow = SkTAddOffset<const void>(srcRow, fSrc.rowBytes());
+        (void) jpeg_write_scanlines(&cinfo, &jpegSrcRow, 1);
+        srcRow = SkTAddOffset<const void>(srcRow, pixmap.rowBytes());
     }
 
-    fCurrRow += numRows;
-    if (fCurrRow == fSrc.height()) {
-        jpeg_finish_compress(fEncoderMgr->cinfo());
-    }
+    jpeg_finish_compress(&cinfo);
+    jpeg_destroy_compress(&cinfo);
 
     return true;
 }
-
-bool SkJpegEncoder::Encode(SkWStream* dst, const SkPixmap& src, const Options& options) {
-    auto encoder = SkJpegEncoder::Make(dst, src, options);
-    return encoder && encoder->encodeRows(src.height());
-}
-
 #endif
diff --git a/src/images/SkJpegEncoder.h b/src/images/SkJpegEncoder.h
deleted file mode 100644 (file)
index 0c41acc..0000000
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Copyright 2017 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#ifndef SkJpegEncoder_DEFINED
-#define SkJpegEncoder_DEFINED
-
-#include "SkPixmap.h"
-#include "SkTypes.h"
-
-class SkWStream;
-
-class SkJpegEncoder : SkNoncopyable {
-public:
-
-    // TODO (skbug.com/1501):
-    // Since jpegs are always opaque, this encoder ignores the alpha channel and treats the
-    // pixels as opaque.
-    // Another possible behavior is to blend the pixels onto opaque black.  We'll need to add
-    // an option for this - and an SkTransferFunctionBehavior.
-
-    struct Options {
-        /**
-         * |fQuality| must be in [0, 100] where 0 corresponds to the lowest quality.
-         */
-        int fQuality = 100;
-    };
-
-    /**
-     *  Encode the |src| pixels to the |dst| stream.
-     *  |options| may be used to control the encoding behavior.
-     *
-     *  Returns true on success.  Returns false on an invalid or unsupported |src|.
-     */
-    static bool Encode(SkWStream* dst, const SkPixmap& src, const Options& options);
-
-    /**
-     *  Create a jpeg encoder that will encode the |src| pixels to the |dst| stream.
-     *  |options| may be used to control the encoding behavior.
-     *
-     *  |dst| is unowned but must remain valid for the lifetime of the object.
-     *
-     *  This returns nullptr on an invalid or unsupported |src|.
-     */
-    static std::unique_ptr<SkJpegEncoder> Make(SkWStream* dst, const SkPixmap& src,
-                                               const Options& options);
-
-    /**
-     *  Encode |numRows| rows of input.  If the caller requests more rows than are remaining
-     *  in the src, this will encode all of the remaining rows.  |numRows| must be greater
-     *  than zero.
-     */
-    bool encodeRows(int numRows);
-};
-
-#endif
index 45362db..4a13533 100644 (file)
@@ -20,7 +20,6 @@
 #include "SkImageEncoderPriv.h"
 #include "SkMD5.h"
 #include "SkOSPath.h"
-#include "SkJpegEncoder.h"
 #include "SkPngChunkReader.h"
 #include "SkRandom.h"
 #include "SkStream.h"
@@ -1531,7 +1530,7 @@ static void encode_format(SkDynamicMemoryWStream* stream, const SkPixmap& pixmap
             SkEncodeImageAsPNG(stream, pixmap, opts);
             break;
         case SkEncodedImageFormat::kJPEG:
-            SkJpegEncoder::Encode(stream, pixmap, SkJpegEncoder::Options());
+            SkEncodeImageAsJPEG(stream, pixmap, opts);
             break;
         case SkEncodedImageFormat::kWEBP:
             SkEncodeImageAsWEBP(stream, pixmap, opts);
diff --git a/tests/EncodeTest.cpp b/tests/EncodeTest.cpp
deleted file mode 100644 (file)
index 9db5039..0000000
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Copyright 2017 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "Resources.h"
-#include "Test.h"
-
-#include "SkBitmap.h"
-#include "SkJpegEncoder.h"
-#include "SkStream.h"
-
-DEF_TEST(Encode_Jpeg, r) {
-    SkBitmap bitmap;
-    bool success = GetResourceAsBitmap("mandrill_128.png", &bitmap);
-    if (!success) {
-        return;
-    }
-
-    SkPixmap src;
-    success = bitmap.peekPixels(&src);
-    REPORTER_ASSERT(r, success);
-    if (!success) {
-        return;
-    }
-
-    SkDynamicMemoryWStream dst0, dst1, dst2, dst3;
-    success = SkJpegEncoder::Encode(&dst0, src, SkJpegEncoder::Options());
-    REPORTER_ASSERT(r, success);
-
-    auto encoder1 = SkJpegEncoder::Make(&dst1, src, SkJpegEncoder::Options());
-    for (int i = 0; i < src.height(); i++) {
-        success = encoder1->encodeRows(1);
-        REPORTER_ASSERT(r, success);
-    }
-
-    auto encoder2 = SkJpegEncoder::Make(&dst2, src, SkJpegEncoder::Options());
-    for (int i = 0; i < src.height(); i+=3) {
-        success = encoder2->encodeRows(3);
-        REPORTER_ASSERT(r, success);
-    }
-
-    auto encoder3 = SkJpegEncoder::Make(&dst3, src, SkJpegEncoder::Options());
-    success = encoder3->encodeRows(200);
-    REPORTER_ASSERT(r, success);
-
-    sk_sp<SkData> data0 = dst0.detachAsData();
-    sk_sp<SkData> data1 = dst1.detachAsData();
-    sk_sp<SkData> data2 = dst2.detachAsData();
-    sk_sp<SkData> data3 = dst3.detachAsData();
-    REPORTER_ASSERT(r, data0->equals(data1.get()));
-    REPORTER_ASSERT(r, data0->equals(data2.get()));
-    REPORTER_ASSERT(r, data0->equals(data3.get()));
-}
index 9363e71..9cb9200 100644 (file)
@@ -530,7 +530,7 @@ static const void* five_reference_pixels(SkColorType colorType) {
 
 static void test_conversion(skiatest::Reporter* r, const SkImageInfo& dstInfo,
                             const SkImageInfo& srcInfo) {
-    if (!SkImageInfoIsValidRenderingCS(srcInfo)) {
+    if (!SkImageInfoIsValid(srcInfo)) {
         return;
     }