Fix decoding GIF to 565

565 cannot take the !writeTransparentPixels path, so disable it for
cases where we might have to take that path.

This only affects frames beyond the first. If the first frame has
a transparent pixel, it will be marked as non-opaque, so we cannot
decode to 565 anyway.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2441833002

Review-Url: https://codereview.chromium.org/2441833002

2 files changed, 36 insertions, 8 deletions

diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp
index c40c27203e..eeaee68c1d 100644
--- a/third_party/gif/SkGifImageReader.cpp
+++ b/third_party/gif/SkGifImageReader.cpp
@@ -361,7 +361,7 @@ sk_sp<SkColorTable> SkGifImageReader::getColorTable(SkColorType colorType, size_
 // Perform decoding for this frame. frameComplete will be true if the entire frame is decoded.
 // Returns false if a decoding error occurred. This is a fatal error and causes the SkGifImageReader to set the "decode failed" flag.
 // Otherwise, either not enough data is available to decode further than before, or the new data has been decoded successfully; returns true in this case.
-bool SkGIFFrameContext::decode(SkGifCodec* client, bool* frameComplete)
+bool SkGIFFrameContext::decode(SkGifCodec* client, const SkGIFColorMap& globalMap, bool* frameComplete)
 {
     *frameComplete = false;
     if (!m_lzwContext) {
@@ -370,7 +370,7 @@ bool SkGIFFrameContext::decode(SkGifCodec* client, bool* frameComplete)
             return true;
 
         m_lzwContext.reset(new SkGIFLZWContext(client, this));
-        if (!m_lzwContext->prepareToDecode()) {
+        if (!m_lzwContext->prepareToDecode(globalMap)) {
             m_lzwContext.reset();
             return false;
         }
@@ -403,7 +403,7 @@ bool SkGifImageReader::decode(size_t frameIndex, bool* frameComplete)
 {
     SkGIFFrameContext* currentFrame = m_frames[frameIndex].get();
 
-    return currentFrame->decode(m_client, frameComplete);
+    return currentFrame->decode(m_client, m_globalColorMap, frameComplete);
 }
 
 // Parse incoming GIF data stream into internal data structures.
@@ -890,7 +890,7 @@ void SkGifImageReader::addFrameIfNecessary()
 }
 
 // FIXME: Move this method to close to doLZW().
-bool SkGIFLZWContext::prepareToDecode()
+bool SkGIFLZWContext::prepareToDecode(const SkGIFColorMap& globalMap)
 {
     SkASSERT(m_frameContext->isDataSizeDefined() && m_frameContext->isHeaderDefined());
 
@@ -906,8 +906,35 @@ bool SkGIFLZWContext::prepareToDecode()
     datum = bits = 0;
     ipass = m_frameContext->interlaced() ? 1 : 0;
     irow = 0;
-    alwaysWriteTransparentPixels = !m_frameContext->interlaced()
-            && m_frameContext->getRequiredFrame() == SkCodec::kNone;
+    alwaysWriteTransparentPixels = false;
+    if (m_frameContext->getRequiredFrame() == SkCodec::kNone) {
+        if (!m_frameContext->interlaced()) {
+            alwaysWriteTransparentPixels = true;
+        } else {
+            // The frame is interlaced, so we do not want to write transparent
+            // pixels. But if there are no transparent pixels anyway, there is
+            // no harm in taking the alwaysWriteTransparentPixels path, which
+            // is faster, and it also supports 565.
+            // Since the frame is independent, it does not matter whether the
+            // frame is subset (nothing behind it needs to show through). So we
+            // only need to know whether there is a valid transparent pixel.
+            // This is a little counterintuitive - we want to "always write
+            // transparent pixels" if there ARE NO transparent pixels, so we
+            // check to see whether the pixel index is >= numColors.
+            const auto& localMap = m_frameContext->localColorMap();
+            const auto trans = m_frameContext->transparentPixel();
+            if (localMap.isDefined()) {
+                alwaysWriteTransparentPixels = trans >= localMap.numColors();
+            } else {
+                // Note that if the map is not defined, the value of
+                // alwaysWriteTransparentPixels is meaningless, since without
+                // any color table, we will skip drawing entirely.
+                // FIXME: We could even skip calling prepareToDecode in that
+                // case, meaning we can SkASSERT(globalMap.isDefined())
+                alwaysWriteTransparentPixels = trans >= globalMap.numColors();
+            }
+        }
+    }
 
     // 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 fee1a5f3c7..5936350681 100644
--- a/third_party/gif/SkGifImageReader.h
+++ b/third_party/gif/SkGifImageReader.h
@@ -86,6 +86,7 @@ enum SkGIFState {
 };
 
 struct SkGIFFrameContext;
+class SkGIFColorMap;
 
 // LZW decoder state machine.
 class SkGIFLZWContext final : public SkNoncopyable {
@@ -108,7 +109,7 @@ public:
         , m_frameContext(frameContext)
     { }
 
-    bool prepareToDecode();
+    bool prepareToDecode(const SkGIFColorMap& globalMap);
     bool outputRow(const unsigned char* rowBegin);
     bool doLZW(const unsigned char* block, size_t bytesInBlock);
     bool hasRemainingRows() { return SkToBool(rowsRemaining); }
@@ -210,7 +211,7 @@ public:
         m_lzwBlocks.push_back(SkData::MakeWithCopy(data, size));
     }
 
-    bool decode(SkGifCodec* client, bool* frameDecoded);
+    bool decode(SkGifCodec* client, const SkGIFColorMap& globalMap, bool* frameDecoded);
 
     int frameId() const { return m_frameId; }
     void setRect(unsigned x, unsigned y, unsigned width, unsigned height)