Write transparent pixels more often (SkGifCodec)
Writing transparent pixels is faster than the alternative, and we can
skip clearing the frame to transparent. We'll still clear if the image
is incomplete.
I ran
./out/Release/nanobench --images <images> --samples 100 --sourceType image --simpleCodec -v
over the GIFs we have on our bots, and found an average ~13% speedup.
Raw data is on sheet 2 of
https://docs.google.com/spreadsheets/d/19V-t9BfbFw5eiwBTKA1qOBkZbchjlTC5EIz6HFy-6RI/
(the sheet is named WriteTransparentPixels).
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2436183002
Review-Url: https://codereview.chromium.org/2436183002
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index e7d8afd..876d71f 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -293,17 +293,12 @@
// We may need to clear to transparent for one of the following reasons:
// - The frameRect does not cover the full bounds. haveDecodedRow will
// only draw inside the frameRect, so we need to clear the rest.
- // - There is a valid transparent pixel value. (FIXME: I'm assuming
- // writeTransparentPixels will be false in this case, based on
- // Chromium's assumption that it would already be zeroed. If we
- // change that behavior, could we skip Filling here?)
// - The frame is interlaced. There is no obvious way to fill
// afterwards for an incomplete image. (FIXME: Does the first pass
// cover all rows? If so, we do not have to fill here.)
// - There is no color table for this frame. In that case will not
// draw anything, so we need to fill.
if (frameContext->frameRect() != this->getInfo().bounds()
- || frameContext->transparentPixel() < SK_MAX_COLORS
|| frameContext->interlaced() || !fCurrColorTableIsReal) {
// fill ignores the width (replaces it with the actual, scaled width).
// But we need to scale in Y.
@@ -489,12 +484,10 @@
void* dstLine = SkTAddOffset<void>(fDst, dstRow * fDstRowBytes);
// We may or may not need to write transparent pixels to the buffer.
- // If we're compositing against a previous image, it's wrong, and if
- // we're writing atop a cleared, fully transparent buffer, it's
- // unnecessary; but if we're decoding an interlaced gif and
- // displaying it "Haeberli"-style, we must write these for passes
- // beyond the first, or the initial passes will "show through" the
- // later ones.
+ // If we're compositing against a previous image, it's wrong, but if
+ // we're decoding an interlaced gif and displaying it "Haeberli"-style,
+ // we must write these for passes beyond the first, or the initial passes
+ // will "show through" the later ones.
const auto dstInfo = this->dstInfo();
if (writeTransparentPixels || dstInfo.colorType() == kRGB_565_SkColorType) {
fSwizzler->swizzle(dstLine, rowBegin);
diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp
index 228c8ec..c40c272 100644
--- a/third_party/gif/SkGifImageReader.cpp
+++ b/third_party/gif/SkGifImageReader.cpp
@@ -146,9 +146,11 @@
if ((unsigned)drowStart >= m_frameContext->height())
return true;
+ bool writeTransparentPixels = alwaysWriteTransparentPixels ||
+ (m_frameContext->progressiveDisplay() && m_frameContext->interlaced() && ipass > 1);
// CALLBACK: Let the client know we have decoded a row.
if (!m_client->haveDecodedRow(m_frameContext->frameId(), rowBegin,
- drowStart, drowEnd - drowStart + 1, m_frameContext->progressiveDisplay() && m_frameContext->interlaced() && ipass > 1))
+ drowStart, drowEnd - drowStart + 1, writeTransparentPixels))
return false;
if (!m_frameContext->interlaced())
@@ -904,6 +906,8 @@
datum = bits = 0;
ipass = m_frameContext->interlaced() ? 1 : 0;
irow = 0;
+ alwaysWriteTransparentPixels = !m_frameContext->interlaced()
+ && m_frameContext->getRequiredFrame() == SkCodec::kNone;
// We want to know the longest sequence encodable by a dictionary with
// SK_MAX_DICTIONARY_ENTRIES entries. If we ignore the need to encode the base
diff --git a/third_party/gif/SkGifImageReader.h b/third_party/gif/SkGifImageReader.h
index 45a1ce6..fee1a5f 100644
--- a/third_party/gif/SkGifImageReader.h
+++ b/third_party/gif/SkGifImageReader.h
@@ -102,6 +102,7 @@
, ipass(0)
, irow(0)
, rowsRemaining(0)
+ , alwaysWriteTransparentPixels(false)
, rowIter(0)
, m_client(client)
, m_frameContext(frameContext)
@@ -125,6 +126,10 @@
int ipass; // Interlace pass; Ranges 1-4 if interlaced.
size_t irow; // Current output row, starting at zero.
size_t rowsRemaining; // Rows remaining to be output.
+ // This depends on the GIFFrameContext. If the frame is not
+ // interlaced and it is independent, it is always safe to
+ // write transparent pixels.
+ bool alwaysWriteTransparentPixels;
unsigned short prefix[SK_MAX_DICTIONARY_ENTRIES];
unsigned char suffix[SK_MAX_DICTIONARY_ENTRIES];