Fix webp bug compositing alpha frames on opaque (better fix)
The old code made the wrong assumptions about premultiplication.
There are three relevant steps here for decoding a webp frame:
1 tell libwebp to decode
2 colorXform the result (sometimes)
3 blend with the prior frame (sometimes)
Rearrange the code to premultiply at the blend step, in a linear space.
If the client wants unpremul, the blend step will unpremul after.
If there is no blending, the colorXform (if any) will premultiply.
If only step 1 is necessary, let libwebp premultiply.
This fixes an animated image that has an opaque frame 0 followed by a
frame with alpha that blends with it.
Add the test image that failed (https://mathiasbynens.be/demo/animated-webp)
The prior fix is in 42bae8faa4b9b6a3341b15c6ac7c6b466e95625c. It did
not properly handle the colorXform when there was no blending step.
Change-Id: I2b9d265ba162eaf7e55a106c8f79341826cee0d3
Reviewed-on: https://skia-review.googlesource.com/72281
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index 6287617..d5614d6 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -39,10 +39,6 @@
return bytesRead >= 14 && !memcmp(bytes, "RIFF", 4) && !memcmp(&bytes[8], "WEBPVP", 6);
}
-static SkAlphaType alpha_type(bool hasAlpha) {
- return hasAlpha ? kUnpremul_SkAlphaType : kOpaque_SkAlphaType;
-}
-
// Parse headers of RIFF container, and check for valid Webp (VP8) content.
// Returns an SkWebpCodec on success
std::unique_ptr<SkCodec> SkWebpCodec::MakeFromStream(std::unique_ptr<SkStream> stream,
@@ -181,9 +177,8 @@
&& dim.height() >= 1 && dim.height() <= info.height();
}
-static WEBP_CSP_MODE webp_decode_mode(const SkImageInfo& info) {
- const bool premultiply = info.alphaType() == kPremul_SkAlphaType;
- switch (info.colorType()) {
+static WEBP_CSP_MODE webp_decode_mode(SkColorType dstCT, bool premultiply) {
+ switch (dstCT) {
case kBGRA_8888_SkColorType:
return premultiply ? MODE_bgrA : MODE_BGRA;
case kRGBA_8888_SkColorType:
@@ -351,20 +346,13 @@
}
}
+// Requires that the src input be unpremultiplied (or opaque).
static void blend_line(SkColorType dstCT, void* dst,
SkColorType srcCT, const void* src,
- bool needsSrgbToLinear, SkAlphaType at,
+ bool needsSrgbToLinear,
+ SkAlphaType dstAt,
+ bool srcHasAlpha,
int width) {
- // Setup conversion from the source and dest, which will be the same.
- SkRasterPipeline_<256> convert_to_linear_premul;
- if (needsSrgbToLinear) {
- convert_to_linear_premul.append_from_srgb(at);
- }
- if (kUnpremul_SkAlphaType == at) {
- // srcover assumes premultiplied inputs.
- convert_to_linear_premul.append(SkRasterPipeline::premul);
- }
-
SkJumper_MemoryCtx dst_ctx = { (void*)dst, 0 },
src_ctx = { (void*)src, 0 };
@@ -374,19 +362,29 @@
// Load the final dst.
p.append(load_dst, &dst_ctx);
- p.extend(convert_to_linear_premul);
+ if (needsSrgbToLinear) {
+ p.append_from_srgb(dstAt);
+ }
+ if (kUnpremul_SkAlphaType == dstAt) {
+ p.append(SkRasterPipeline::premul);
+ }
p.append(SkRasterPipeline::move_src_dst);
// Load the src.
SkRasterPipeline::StockStage load_src;
pick_memory_stages(srcCT, &load_src, nullptr);
p.append(load_src, &src_ctx);
- p.extend(convert_to_linear_premul);
+ if (needsSrgbToLinear) {
+ p.append_from_srgb(kUnpremul_SkAlphaType);
+ }
+ if (srcHasAlpha) {
+ p.append(SkRasterPipeline::premul);
+ }
p.append(SkRasterPipeline::srcover);
// Convert back to dst.
- if (kUnpremul_SkAlphaType == at) {
+ if (kUnpremul_SkAlphaType == dstAt) {
p.append(SkRasterPipeline::unpremul);
}
if (needsSrgbToLinear) {
@@ -537,7 +535,35 @@
webpDst.installPixels(webpInfo, dst, rowBytes);
}
- config.output.colorspace = webp_decode_mode(webpInfo);
+ // Choose the step when we will perform premultiplication.
+ enum {
+ kNone,
+ kBlendLine,
+ kColorXform,
+ kLibwebp,
+ };
+ auto choose_premul_step = [&]() {
+ if (!frame.has_alpha) {
+ // None necessary.
+ return kNone;
+ }
+ if (blendWithPrevFrame) {
+ // Premultiply in blend_line, in a linear space.
+ return kBlendLine;
+ }
+ if (dstInfo.alphaType() != kPremul_SkAlphaType) {
+ // No blending is necessary, so we only need to premultiply if the
+ // client requested it.
+ return kNone;
+ }
+ if (this->colorXform()) {
+ // Premultiply in the colorXform, in a linear space.
+ return kColorXform;
+ }
+ return kLibwebp;
+ };
+ const auto premulStep = choose_premul_step();
+ config.output.colorspace = webp_decode_mode(webpInfo.colorType(), premulStep == kLibwebp);
config.output.is_external_memory = 1;
config.output.u.RGBA.rgba = reinterpret_cast<uint8_t*>(webpDst.getAddr(dstX, dstY));
@@ -568,10 +594,6 @@
return kInvalidInput;
}
- // We're only transforming the new part of the frame, so no need to worry about the
- // final composited alpha.
- const auto srcAlpha = 0 == index ? srcInfo.alphaType() : alpha_type(frame.has_alpha);
- const auto xformAlphaType = select_xform_alpha(dstInfo.alphaType(), srcAlpha);
const bool needsSrgbToLinear = dstInfo.gammaCloseToSRGB() &&
options.fPremulBehavior == SkTransferFunctionBehavior::kRespect;
@@ -592,11 +614,15 @@
} else {
xformDst = dst;
}
+
+ const auto xformAlphaType = (premulStep == kColorXform) ? kPremul_SkAlphaType :
+ ( frame.has_alpha) ? kUnpremul_SkAlphaType :
+ kOpaque_SkAlphaType ;
for (int y = 0; y < rowsDecoded; y++) {
this->applyColorXform(xformDst, xformSrc, scaledWidth, xformAlphaType);
if (blendWithPrevFrame) {
- blend_line(dstCT, dst, dstCT, xformDst, needsSrgbToLinear, xformAlphaType,
- scaledWidth);
+ blend_line(dstCT, dst, dstCT, xformDst, needsSrgbToLinear,
+ dstInfo.alphaType(), frame.has_alpha, scaledWidth);
dst = SkTAddOffset<void>(dst, rowBytes);
} else {
xformDst = SkTAddOffset<void>(xformDst, rowBytes);
@@ -608,7 +634,7 @@
for (int y = 0; y < rowsDecoded; y++) {
blend_line(dstCT, dst, webpDst.colorType(), src, needsSrgbToLinear,
- xformAlphaType, scaledWidth);
+ dstInfo.alphaType(), frame.has_alpha, scaledWidth);
src = SkTAddOffset<const uint8_t>(src, srcRowBytes);
dst = SkTAddOffset<void>(dst, rowBytes);
}