From e7f165be2e2d6208110c257bb42ff20127821211 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Mon, 10 Dec 2018 10:29:18 -0500 Subject: [PATCH] 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 --- BUILD.gn | 1 + include/core/SkImageEncoder.h | 6 +++-- src/images/SkImageEncoder.cpp | 21 +++++++++++++-- tests/EncodeTest.cpp | 48 +++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 5b6f7e2bc9..9d3c5c01eb 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1560,6 +1560,7 @@ 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 a6d7cb8c68..b01771aa82 100644 --- a/include/core/SkImageEncoder.h +++ b/include/core/SkImageEncoder.h @@ -27,7 +27,8 @@ * Will always return false if Skia is compiled without image * encoders. * - * Note that webp encodes will use webp lossy compression. + * For SkEncodedImageFormat::kWEBP, if quality is 100, it will use lossless compression. Otherwise + * it will use lossy. * * For examples of encoding an image to a file or to a block of memory, * see tools/sk_tool_utils.h. @@ -56,7 +57,8 @@ inline bool SkEncodeImage(SkWStream* dst, const SkBitmap& src, SkEncodedImageFor * Will always return nullptr if Skia is compiled without image * encoders. * - * Note that webp encodes will use webp lossy compression. + * For SkEncodedImageFormat::kWEBP, if quality is 100, it will use lossless compression. Otherwise + * it will use lossy. */ 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 946a67f231..d5cf2ec623 100644 --- a/src/images/SkImageEncoder.cpp +++ b/src/images/SkImageEncoder.cpp @@ -48,8 +48,25 @@ bool SkEncodeImage(SkWStream* dst, const SkPixmap& src, } case SkEncodedImageFormat::kWEBP: { SkWebpEncoder::Options opts; - opts.fCompression = SkWebpEncoder::Compression::kLossy; - opts.fQuality = quality; + 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; + } return SkWebpEncoder::Encode(dst, src, opts); } default: diff --git a/tests/EncodeTest.cpp b/tests/EncodeTest.cpp index 30b53022df..5dab124b9f 100644 --- a/tests/EncodeTest.cpp +++ b/tests/EncodeTest.cpp @@ -23,6 +23,8 @@ #include #include +#include "webp/decode.h" + static bool encode(SkEncodedImageFormat format, SkWStream* dst, const SkPixmap& src) { switch (format) { case SkEncodedImageFormat::kJPEG: @@ -286,6 +288,52 @@ 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);