From 1285f413950910782d5439b5072ccfa14bdf80f7 Mon Sep 17 00:00:00 2001 From: scroggo Date: Wed, 26 Oct 2016 13:48:03 -0700 Subject: [PATCH] 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 --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 --- src/codec/SkGifCodec.cpp | 15 ++++----------- third_party/gif/SkGifImageReader.cpp | 6 +++++- third_party/gif/SkGifImageReader.h | 5 +++++ 3 files changed, 14 insertions(+), 12 deletions(-) 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 @@ SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts, // 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 @@ bool SkGifCodec::haveDecodedRow(size_t frameIndex, const unsigned char* rowBegin void* dstLine = SkTAddOffset(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 @@ bool SkGIFLZWContext::outputRow(const unsigned char* rowBegin) 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 @@ bool SkGIFLZWContext::prepareToDecode() 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 @@ public: , ipass(0) , irow(0) , rowsRemaining(0) + , alwaysWriteTransparentPixels(false) , rowIter(0) , m_client(client) , m_frameContext(frameContext) @@ -125,6 +126,10 @@ private: 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]; -- 2.7.4