SkWuffsCodec: zero dst buffer before decodeFrame
https://skia-review.googlesource.com/c/189866 "SkWuffsCodec: Initialize
memory when incomplete" made some buffer zero'ing conditional on a
frame's dirty rectangle, which required moving the this->decodeFrame
call earlier in onIncrementalDecode, since we can't know the dirty
rectangle until after decodeFrame is called.
However, moving that earlier meant that it was now earlier than the
"sk_bzero(pixels.ptr + etc, etc)" call that zero-initialized the
destination buffer that decodeFrame writes to. The actual pixel indexes
that decodeFrame decodes are now overwritten by zeroes.
That prior commit fixed the fuzzer-discovered bug, in that it no longer
swizzles from uninitialized memory. Unfortunately, it's now often
swizzling from all-zeroed memory, so that the decoding is incorrect.
This commit moves the zero-ing to onStartIncrementalDecode, even
earlier. The "dm --match Gif" tests are happy again.
Bug: skia:8235
Change-Id: Iae993b7d9f45e1b375ed99f6cc86536091a92eba
Reviewed-on: https://skia-review.googlesource.com/c/190941
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
diff --git a/src/codec/SkWuffsCodec.cpp b/src/codec/SkWuffsCodec.cpp
index 04da11c..8384c1c 100644
--- a/src/codec/SkWuffsCodec.cpp
+++ b/src/codec/SkWuffsCodec.cpp
@@ -395,16 +395,27 @@
fColorTableFilled = false;
const char* status = this->decodeFrameConfig();
- if (status == nullptr) {
- fIncrDecDst = static_cast<uint8_t*>(dst);
- fIncrDecRowBytes = rowBytes;
- return SkCodec::kSuccess;
- } else if (status == wuffs_base__suspension__short_read) {
+ if (status == wuffs_base__suspension__short_read) {
return SkCodec::kIncompleteInput;
- } else {
+ } else if (status != nullptr) {
SkCodecPrintf("decodeFrameConfig: %s", status);
return SkCodec::kErrorInInput;
}
+
+ // In Wuffs, a paletted image is always 1 byte per pixel.
+ static constexpr size_t src_bpp = 1;
+
+ // Zero-initialize Wuffs' buffer covering the frame rect.
+ wuffs_base__rect_ie_u32 frame_rect = fFrameConfig.bounds();
+ wuffs_base__table_u8 pixels = fPixelBuffer.plane(0);
+ for (uint32_t y = frame_rect.min_incl_y; y < frame_rect.max_excl_y; y++) {
+ sk_bzero(pixels.ptr + (y * pixels.stride) + (frame_rect.min_incl_x * src_bpp),
+ frame_rect.width() * src_bpp);
+ }
+
+ fIncrDecDst = static_cast<uint8_t*>(dst);
+ fIncrDecRowBytes = rowBytes;
+ return SkCodec::kSuccess;
}
static bool independent_frame(SkCodec* codec, int frameIndex) {
@@ -465,16 +476,8 @@
fSwizzler->setSampleY(fSpySampler.sampleY());
scaledHeight = get_scaled_dimension(dstInfo().height(), fSpySampler.sampleY());
- // Zero-initialize wuffs' buffer covering the frame rect. This will later be used to
- // determine how we write to the output, even if the image was incomplete. This ensures
- // that we do not swizzle uninitialized memory.
- for (uint32_t y = frame_rect.min_incl_y; y < frame_rect.max_excl_y; y++) {
- uint8_t* s = pixels.ptr + (y * pixels.stride) + (frame_rect.min_incl_x * src_bpp);
- sk_bzero(s, frame_rect.width() * src_bpp);
- }
-
// If the frame rect does not fill the output, ensure that those pixels are not
- // left uninitialized either.
+ // left uninitialized.
if (independent && (bounds != this->bounds() || dirty_rect.is_empty())) {
auto fillInfo = dstInfo().makeWH(fSwizzler->fillWidth(), scaledHeight);
SkSampler::Fill(fillInfo, fIncrDecDst, fIncrDecRowBytes, options().fZeroInitialized);