Write transparent pixels more often (SkGifCodec)
authorscroggo <scroggo@chromium.org>
Wed, 26 Oct 2016 20:48:03 +0000 (13:48 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 26 Oct 2016 20:48:03 +0000 (13:48 -0700)
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

src/codec/SkGifCodec.cpp
third_party/gif/SkGifImageReader.cpp
third_party/gif/SkGifImageReader.h

index e7d8afd..876d71f 100644 (file)
@@ -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<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);
index 228c8ec..c40c272 100644 (file)
@@ -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
index 45a1ce6..fee1a5f 100644 (file)
@@ -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];