Decode GIF image frames on demand.

Source/core/platform/image-decoders/gif/GIFImageDecoder.cpp

Index: Source/core/platform/image-decoders/gif/GIFImageDecoder.cpp
diff --git a/Source/core/platform/image-decoders/gif/GIFImageDecoder.cpp b/Source/core/platform/image-decoders/gif/GIFImageDecoder.cpp
index d785aec0341c6f5f782e4ffe3b97e4b74b001224..ba8306e17e27d1edaec27e59e796aa250ad275a3 100644
--- a/Source/core/platform/image-decoders/gif/GIFImageDecoder.cpp
+++ b/Source/core/platform/image-decoders/gif/GIFImageDecoder.cpp
@@ -29,7 +29,8 @@
 #include <limits>
 #include "core/platform/PlatformInstrumentation.h"
 #include "core/platform/image-decoders/gif/GIFImageReader.h"
-#include <wtf/PassOwnPtr.h>
+#include "wtf/NotFound.h"
+#include "wtf/PassOwnPtr.h"
 
 namespace WebCore {
 
@@ -57,7 +58,7 @@ void GIFImageDecoder::setData(SharedBuffer* data, bool allDataReceived)
 bool GIFImageDecoder::isSizeAvailable()
 {
     if (!ImageDecoder::isSizeAvailable())
-         decode(0, GIFSizeQuery);
+        parse(GIFSizeQuery);
 
     return ImageDecoder::isSizeAvailable();
 }
@@ -76,7 +77,7 @@ bool GIFImageDecoder::setSize(unsigned width, unsigned height)
 
 size_t GIFImageDecoder::frameCount()
 {
-    decode(std::numeric_limits<unsigned>::max(), GIFFrameCountQuery);
+    parse(GIFFrameCountQuery);
     return m_frameBufferCache.size();
 }
 
@@ -121,7 +122,7 @@ ImageFrame* GIFImageDecoder::frameBufferAtIndex(size_t index)
     ImageFrame& frame = m_frameBufferCache[index];
     if (frame.status() != ImageFrame::FrameComplete) {
         PlatformInstrumentation::willDecodeImage("GIF");
-        decode(index + 1, GIFFullQuery);
+        decode(index);
         PlatformInstrumentation::didDecodeImage();
     }
     return &frame;
@@ -145,57 +146,7 @@ bool GIFImageDecoder::setFailed()
     return ImageDecoder::setFailed();
 }
 
-void GIFImageDecoder::clearFrameBufferCache(size_t clearBeforeFrame)
-{
-    // In some cases, like if the decoder was destroyed while animating, we
-    // can be asked to clear more frames than we currently have.
-    if (m_frameBufferCache.isEmpty())
-        return; // Nothing to do.
-
-    // The "-1" here is tricky.  It does not mean that |clearBeforeFrame| is the
-    // last frame we wish to preserve, but rather that we never want to clear
-    // the very last frame in the cache: it's empty (so clearing it is
-    // pointless), it's partial (so we don't want to clear it anyway), or the
-    // cache could be enlarged with a future setData() call and it could be
-    // needed to construct the next frame (see comments below).  Callers can
-    // always use ImageSource::clear(true, ...) to completely free the memory in
-    // this case.
-    clearBeforeFrame = std::min(clearBeforeFrame, m_frameBufferCache.size() - 1);
-    const Vector<ImageFrame>::iterator end(m_frameBufferCache.begin() + clearBeforeFrame);
-
-    // We need to preserve frames such that:
-    //   * We don't clear |end|
-    //   * We don't clear the frame we're currently decoding
-    //   * We don't clear any frame from which a future initFrameBuffer() call
-    //     will copy bitmap data
-    // All other frames can be cleared.  Because of the constraints on when
-    // ImageSource::clear() can be called (see ImageSource.h), we're guaranteed
-    // not to have non-empty frames after the frame we're currently decoding.
-    // So, scan backwards from |end| as follows:
-    //   * If the frame is empty, we're still past any frames we care about.
-    //   * If the frame is complete, but is DisposeOverwritePrevious, we'll
-    //     skip over it in future initFrameBuffer() calls.  We can clear it
-    //     unless it's |end|, and keep scanning.  For any other disposal method,
-    //     stop scanning, as we've found the frame initFrameBuffer() will need
-    //     next.
-    //   * If the frame is partial, we're decoding it, so don't clear it; if it
-    //     has a disposal method other than DisposeOverwritePrevious, stop
-    //     scanning, as we'll only need this frame when decoding the next one.
-    Vector<ImageFrame>::iterator i(end);
-    for (; (i != m_frameBufferCache.begin()) && ((i->status() == ImageFrame::FrameEmpty) || (i->disposalMethod() == ImageFrame::DisposeOverwritePrevious)); --i) {
-        if ((i->status() == ImageFrame::FrameComplete) && (i != end))
-            i->clearPixelData();
-    }
-
-    // Now |i| holds the last frame we need to preserve; clear prior frames.
-    for (Vector<ImageFrame>::iterator j(m_frameBufferCache.begin()); j != i; ++j) {
-        ASSERT(j->status() != ImageFrame::FramePartial);
-        if (j->status() != ImageFrame::FrameEmpty)
-            j->clearPixelData();
-    }
-}
-
-bool GIFImageDecoder::haveDecodedRow(unsigned frameIndex, const Vector<unsigned char>& rowBuffer, size_t width, size_t rowNumber, unsigned repeatCount, bool writeTransparentPixels)
+bool GIFImageDecoder::haveDecodedRow(size_t frameIndex, const Vector<unsigned char>& rowBuffer, size_t width, size_t rowNumber, unsigned repeatCount, bool writeTransparentPixels)
 {
     const GIFFrameContext* frameContext = m_reader->frameContext(frameIndex);
     // The pixel data and coordinates supplied to us are relative to the frame's
@@ -258,7 +209,12 @@ bool GIFImageDecoder::haveDecodedRow(unsigned frameIndex, const Vector<unsigned
     return true;
 }
 
-bool GIFImageDecoder::frameComplete(unsigned frameIndex, unsigned frameDuration, ImageFrame::FrameDisposalMethod disposalMethod)
+bool GIFImageDecoder::parseCompleted() const
+{
+    return m_reader && m_reader->parseCompleted();
+}
+
+bool GIFImageDecoder::frameComplete(size_t frameIndex)
 {
     // Initialize the frame if necessary.  Some GIFs insert do-nothing frames,
     // in which case we never reach haveDecodedRow() before getting here.
@@ -267,26 +223,20 @@ bool GIFImageDecoder::frameComplete(unsigned frameIndex, unsigned frameDuration,
         return false; // initFrameBuffer() has already called setFailed().
 
     buffer.setStatus(ImageFrame::FrameComplete);
-    buffer.setDuration(frameDuration);
-    buffer.setDisposalMethod(disposalMethod);
 
     if (!m_currentBufferSawAlpha) {
         // The whole frame was non-transparent, so it's possible that the entire
         // resulting buffer was non-transparent, and we can setHasAlpha(false).
-        if (buffer.originalFrameRect().contains(IntRect(IntPoint(), scaledSize())))
+        if (buffer.originalFrameRect().contains(IntRect(IntPoint(), scaledSize()))) {
             buffer.setHasAlpha(false);
-        else if (frameIndex) {
+            buffer.setRequiredPreviousFrameIndex(notFound);
+        } else if (buffer.requiredPreviousFrameIndex() != notFound) {
             // Tricky case.  This frame does not have alpha only if everywhere
             // outside its rect doesn't have alpha.  To know whether this is
             // true, we check the start state of the frame -- if it doesn't have
             // alpha, we're safe.
-            //
-            // First skip over prior DisposeOverwritePrevious frames (since they
-            // don't affect the start state of this frame) the same way we do in
-            // initFrameBuffer().
-            const ImageFrame* prevBuffer = &m_frameBufferCache[--frameIndex];
-            while (frameIndex && (prevBuffer->disposalMethod() == ImageFrame::DisposeOverwritePrevious))
-                prevBuffer = &m_frameBufferCache[--frameIndex];
+            const ImageFrame* prevBuffer = &m_frameBufferCache[buffer.requiredPreviousFrameIndex()];
+            ASSERT(prevBuffer->disposalMethod() != ImageFrame::DisposeOverwritePrevious);
 
             // Now, if we're at a DisposeNotSpecified or DisposeKeep frame, then
             // we can say we have no alpha if that frame had no alpha.  But
@@ -304,14 +254,17 @@ bool GIFImageDecoder::frameComplete(unsigned frameIndex, unsigned frameDuration,
     return true;
 }
 
-void GIFImageDecoder::gifComplete()
+void GIFImageDecoder::clearFrameBuffer(size_t frameIndex)
 {
-    // Cache the repetition count, which is now as authoritative as it's ever
-    // going to be.
-    repetitionCount();
+    if (m_reader && m_frameBufferCache[frameIndex].status() == ImageFrame::FramePartial) {
+        // Reset the states of the partial frame in the reader so that the frame

[Peter Kasting, 2013/05/24 03:15:22]

Nit: states -> state

[Xianzhu, 2013/05/28 22:54:21]

Done.

+        // can be decoded again when requested.
+        m_reader->clearDecodeState(frameIndex);
+    }
+    ImageDecoder::clearFrameBuffer(frameIndex);
 }
 
-void GIFImageDecoder::decode(unsigned haltAtFrame, GIFQuery query)
+void GIFImageDecoder::parse(GIFParseQuery query)
 {
     if (failed())
         return;
@@ -321,37 +274,57 @@ void GIFImageDecoder::decode(unsigned haltAtFrame, GIFQuery query)
         m_reader->setData(m_data);
     }
 
-    if (query == GIFSizeQuery) {
-        if (!m_reader->decode(GIFSizeQuery, haltAtFrame))
-            setFailed();
-        return;
-    }
-
-    if (!m_reader->decode(GIFFrameCountQuery, haltAtFrame)) {
+    if (!m_reader->parse(query)) {
         setFailed();
         return;
     }
 
     const size_t oldSize = m_frameBufferCache.size();
     m_frameBufferCache.resize(m_reader->imagesCount());
-    for (size_t i = oldSize; i < m_reader->imagesCount(); ++i)
-        m_frameBufferCache[i].setPremultiplyAlpha(m_premultiplyAlpha);
 
-    if (query == GIFFrameCountQuery)
-        return;
+    for (size_t i = oldSize; i < m_reader->imagesCount(); ++i) {
+        ImageFrame& buffer = m_frameBufferCache[i];
+        const GIFFrameContext* frameContext = m_reader->frameContext(i);
+        buffer.setPremultiplyAlpha(m_premultiplyAlpha);
+        buffer.setRequiredPreviousFrameIndex(findRequiredPreviousFrame(i));
+        buffer.setDuration(frameContext->delayTime);
+        buffer.setDisposalMethod(frameContext->disposalMethod);
+    }
+}
 
-    if (!m_reader->decode(GIFFullQuery, haltAtFrame)) {
-        setFailed();
+void GIFImageDecoder::decode(size_t frameIndex)
+{
+    parse(GIFFrameCountQuery);
+
+    if (failed())
         return;
+
+    Vector<size_t> framesToDecode;
+    size_t frameToDecode = frameIndex;
+    do {
+        framesToDecode.append(frameToDecode);
+        frameToDecode = m_frameBufferCache[frameToDecode].requiredPreviousFrameIndex();
+    } while (frameToDecode != notFound && m_frameBufferCache[frameToDecode].status() == ImageFrame::FrameEmpty);

[Peter Kasting, 2013/05/24 03:15:22]

Doesn't this mean if frame 1 is FramePartial, we ask for frame 2, and frame 2
needs frame 1, we'll only try to decode frame 2, without first finishing
decoding frame 1?

[Xianzhu, 2013/05/28 22:54:21]

Thanks for finding the mistake. Corrected.

+
+    for (size_t i = framesToDecode.size(); i; --i) {

[Peter Kasting, 2013/05/24 03:15:22]

Nit: Use a const_reverse_iterator, it's clearer and less error-prone.

[Xianzhu, 2013/05/28 22:54:21]

Done.

+        size_t frameIndex = framesToDecode[i - 1];
+        if (!m_reader->decode(frameIndex)) {
+            setFailed();
+            return;
+        }
+
+        // We need more data to continue decoding.
+        if (m_frameBufferCache[frameIndex].status() != ImageFrame::FrameComplete)
+            break;
     }
 
     // It is also a fatal error if all data is received and we have decoded all
     // frames available but the file is truncated.
-    if (haltAtFrame >= m_frameBufferCache.size() && isAllDataReceived() && m_reader && !m_reader->parseCompleted())
+    if (frameIndex >= m_frameBufferCache.size() - 1 && isAllDataReceived() && m_reader && !m_reader->parseCompleted())
         setFailed();
 }
 
-bool GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
+bool GIFImageDecoder::initFrameBuffer(size_t frameIndex)
 {
     // Initialize the frame rect in our buffer.
     const GIFFrameContext* frameContext = m_reader->frameContext(frameIndex);
@@ -369,53 +342,30 @@ bool GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
     int top = upperBoundScaledY(frameRect.y());
     int bottom = lowerBoundScaledY(frameRect.maxY(), top);
     buffer->setOriginalFrameRect(IntRect(left, top, right - left, bottom - top));
-    
-    if (!frameIndex) {
-        // This is the first frame, so we're not relying on any previous data.
+
+    size_t requiredPreviousFrameIndex = m_frameBufferCache[frameIndex].requiredPreviousFrameIndex();

[Peter Kasting, 2013/05/24 03:15:22]

Nit: Use |buffer|

[Xianzhu, 2013/05/28 22:54:21]

Done.

+    if (requiredPreviousFrameIndex == notFound) {
+        // This frame doesn't rely on any previous data.
         if (!buffer->setSize(scaledSize().width(), scaledSize().height()))
             return setFailed();
     } else {
-        // The starting state for this frame depends on the previous frame's
-        // disposal method.
-        //
-        // Frames that use the DisposeOverwritePrevious method are effectively
-        // no-ops in terms of changing the starting state of a frame compared to
-        // the starting state of the previous frame, so skip over them.  (If the
-        // first frame specifies this method, it will get treated like
-        // DisposeOverwriteBgcolor below and reset to a completely empty image.)
-        const ImageFrame* prevBuffer = &m_frameBufferCache[--frameIndex];
-        ImageFrame::FrameDisposalMethod prevMethod = prevBuffer->disposalMethod();
-        while (frameIndex && (prevMethod == ImageFrame::DisposeOverwritePrevious)) {
-            prevBuffer = &m_frameBufferCache[--frameIndex];
-            prevMethod = prevBuffer->disposalMethod();
-        }
+        const ImageFrame* prevBuffer = &m_frameBufferCache[requiredPreviousFrameIndex];
         ASSERT(prevBuffer->status() == ImageFrame::FrameComplete);
 
-        if ((prevMethod == ImageFrame::DisposeNotSpecified) || (prevMethod == ImageFrame::DisposeKeep)) {
-            // Preserve the last frame as the starting state for this frame.
-            if (!buffer->copyBitmapData(*prevBuffer))
-                return setFailed();
-        } else {
+        // Preserve the last frame as the starting state for this frame.
+        if (!buffer->copyBitmapData(*prevBuffer))

[Peter Kasting, 2013/05/24 03:15:22]

Your code here is simpler than before, but much less efficient in the case where
we have DisposeOverwriteBgcolor and a full frame.  In that case, we should
instead use the setSize() code above to simply clear everything.

Something like:

    if ((requiredPreviousFrameIndex == notFound)
        || ((prevBuffer->disposalMethod() ==
ImageFrame::DisposeOverwriteBgcolor)
            && (prevBuffer->originalFrameRect().contains(IntRect(IntPoint(),
size()))))) {
      // This frame doesn't rely on any previous data.
      ...

...Then I'd put an ASSERT (or at least a comment) here that the prevRect is
smaller than the full image.

[Xianzhu, 2013/05/28 22:54:21]

In the case findRequiredPreviousFrame() should have already set
requiredPreviousFrameIndex to notFound, so we wouldn't need to check it again
here. However, I found a bug that the originalFrameRect is not set when this
method is called. Fixed.

The ASSERT is useful. Added.

+            return setFailed();
+
+        if (prevBuffer->disposalMethod() == ImageFrame::DisposeOverwriteBgcolor) {
             // We want to clear the previous frame to transparent, without
             // affecting pixels in the image outside of the frame.
             const IntRect& prevRect = prevBuffer->originalFrameRect();
-            const IntSize& bufferSize = scaledSize();
-            if (!frameIndex || prevRect.contains(IntRect(IntPoint(), scaledSize()))) {
-                // Clearing the first frame, or a frame the size of the whole
-                // image, results in a completely empty image.
-                if (!buffer->setSize(bufferSize.width(), bufferSize.height()))
-                    return setFailed();
-            } else {
-              // Copy the whole previous buffer, then clear just its frame.
-              if (!buffer->copyBitmapData(*prevBuffer))
-                  return setFailed();
-              for (int y = prevRect.y(); y < prevRect.maxY(); ++y) {
-                  for (int x = prevRect.x(); x < prevRect.maxX(); ++x)
-                      buffer->setRGBA(x, y, 0, 0, 0, 0);
-              }
-              if ((prevRect.width() > 0) && (prevRect.height() > 0))
-                  buffer->setHasAlpha(true);
+            for (int y = prevRect.y(); y < prevRect.maxY(); ++y) {
+                for (int x = prevRect.x(); x < prevRect.maxX(); ++x)
+                    buffer->setRGBA(x, y, 0, 0, 0, 0);
             }
+            if ((prevRect.width() > 0) && (prevRect.height() > 0))
+                buffer->setHasAlpha(true);
         }
     }