From 22170b317800325408310e144194d6c024d571bc Mon Sep 17 00:00:00 2001 From: Mike Reed Date: Tue, 11 Dec 2018 17:12:39 +0000 Subject: [PATCH] Revert "Treat kWEBP encode with quality=100 as lossless" This reverts commit e7f165be2e2d6208110c257bb42ff20127821211. Reason for revert: doesn't compile in google3 Original change's description: > Treat kWEBP encode with quality=100 as lossless > > In SkEncodeImage and friends, treat quality of 100 as a lossless encode > when using kWEBP. This seems a good fit for the intent - which is > presumably to save the highest quality image. This also matches > Chromium's blink::ImageEncoder::ComputeWebpOptions, which treats a > quality of 1 (on a float scale from 0 to 1) as a lossless encode. > > FWIW, Chromium has had this behavior since > https://codereview.chromium.org/1937433002, in response to > crbug.com/523098. The goal is to "maintain sharpness to > match the JPEG encoder behavior (use WEBP lossless encoding)". > > Add a test to verify the new behavior. This requires making tests > depend on libwebp to use WebPGetFeatures, since the Skia API does not > provide a way to determine whether an encoded webp file was encoded > lossless-ly or lossily. > > Bug: skia:8586 > Change-Id: Ie9e09c2f7414ab701d696c4ad9edf405868a716f > Reviewed-on: https://skia-review.googlesource.com/c/175823 > Commit-Queue: Leon Scroggins > Reviewed-by: Derek Sollenberger > Reviewed-by: Mike Reed TBR=djsollen@google.com,scroggo@google.com,reed@google.com Change-Id: I91680f65a2a5e6f0a13b84e97c9541ebe0606b33 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia:8586 Reviewed-on: https://skia-review.googlesource.com/c/176584 Reviewed-by: Mike Reed Commit-Queue: Mike Reed --- BUILD.gn | 1 - include/core/SkImageEncoder.h | 6 ++--- src/images/SkImageEncoder.cpp | 21 ++------------- tests/EncodeTest.cpp | 48 ----------------------------------- 4 files changed, 4 insertions(+), 72 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 9d3c5c01eb..5b6f7e2bc9 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1560,7 +1560,6 @@ if (skia_enable_tools) { "modules/skottie:tests", "modules/sksg:tests", "//third_party/libpng", - "//third_party/libwebp", "//third_party/zlib", ] public_deps = [ diff --git a/include/core/SkImageEncoder.h b/include/core/SkImageEncoder.h index b01771aa82..a6d7cb8c68 100644 --- a/include/core/SkImageEncoder.h +++ b/include/core/SkImageEncoder.h @@ -27,8 +27,7 @@ * Will always return false if Skia is compiled without image * encoders. * - * For SkEncodedImageFormat::kWEBP, if quality is 100, it will use lossless compression. Otherwise - * it will use lossy. + * Note that webp encodes will use webp lossy compression. * * For examples of encoding an image to a file or to a block of memory, * see tools/sk_tool_utils.h. @@ -57,8 +56,7 @@ inline bool SkEncodeImage(SkWStream* dst, const SkBitmap& src, SkEncodedImageFor * Will always return nullptr if Skia is compiled without image * encoders. * - * For SkEncodedImageFormat::kWEBP, if quality is 100, it will use lossless compression. Otherwise - * it will use lossy. + * Note that webp encodes will use webp lossy compression. */ SK_API sk_sp SkEncodePixmap(const SkPixmap& src, SkEncodedImageFormat format, int quality); diff --git a/src/images/SkImageEncoder.cpp b/src/images/SkImageEncoder.cpp index d5cf2ec623..946a67f231 100644 --- a/src/images/SkImageEncoder.cpp +++ b/src/images/SkImageEncoder.cpp @@ -48,25 +48,8 @@ bool SkEncodeImage(SkWStream* dst, const SkPixmap& src, } case SkEncodedImageFormat::kWEBP: { SkWebpEncoder::Options opts; - if (quality == 100) { - opts.fCompression = SkWebpEncoder::Compression::kLossless; - // Note: SkEncodeImage treats 0 quality as the lowest quality - // (greatest compression) and 100 as the highest quality (least - // compression). For kLossy, this matches libwebp's - // interpretation, so it is passed directly to libwebp. But - // with kLossless, libwebp always creates the highest quality - // image. In this case, fQuality is reinterpreted as how much - // effort (time) to put into making a smaller file. This API - // does not provide a way to specify this value (though it can - // be specified by using SkWebpEncoder::Encode) so we have to - // pick one arbitrarily. This value matches that chosen by - // blink::ImageEncoder::ComputeWebpOptions as well - // WebPConfigInit. - opts.fQuality = 75; - } else { - opts.fCompression = SkWebpEncoder::Compression::kLossy; - opts.fQuality = quality; - } + opts.fCompression = SkWebpEncoder::Compression::kLossy; + opts.fQuality = quality; return SkWebpEncoder::Encode(dst, src, opts); } default: diff --git a/tests/EncodeTest.cpp b/tests/EncodeTest.cpp index 5dab124b9f..30b53022df 100644 --- a/tests/EncodeTest.cpp +++ b/tests/EncodeTest.cpp @@ -23,8 +23,6 @@ #include #include -#include "webp/decode.h" - static bool encode(SkEncodedImageFormat format, SkWStream* dst, const SkPixmap& src) { switch (format) { case SkEncodedImageFormat::kJPEG: @@ -288,52 +286,6 @@ DEF_TEST(Encode_PngOptions, r) { REPORTER_ASSERT(r, almost_equals(bm0, bm2, 0)); } -DEF_TEST(Encode_WebpQuality, r) { - SkBitmap bm; - bm.allocN32Pixels(100, 100); - bm.eraseColor(SK_ColorBLUE); - - auto dataLossy = SkEncodeBitmap(bm, SkEncodedImageFormat::kWEBP, 99); - auto dataLossLess = SkEncodeBitmap(bm, SkEncodedImageFormat::kWEBP, 100); - - enum Format { - kMixed = 0, - kLossy = 1, - kLossless = 2, - }; - - auto test = [&r](const sk_sp& data, Format expected) { - auto printFormat = [](int f) { - switch (f) { - case kMixed: return "mixed"; - case kLossy: return "lossy"; - case kLossless: return "lossless"; - default: return "unknown"; - } - }; - - if (!data) { - ERRORF(r, "Failed to encode. Expected %s", printFormat(expected)); - return; - } - - WebPBitstreamFeatures features; - auto status = WebPGetFeatures(data->bytes(), data->size(), &features); - if (status != VP8_STATUS_OK) { - ERRORF(r, "Encode had an error %i. Expected %s", status, printFormat(expected)); - return; - } - - if (expected != features.format) { - ERRORF(r, "Expected %s encode, but got format %s", printFormat(expected), - printFormat(features.format)); - } - }; - - test(dataLossy, kLossy); - test(dataLossLess, kLossless); -} - DEF_TEST(Encode_WebpOptions, r) { SkBitmap bitmap; bool success = GetResourceAsBitmap("images/google_chrome.ico", &bitmap);