Decode GIF image frames on demand.

Source/core/platform/image-decoders/ImageDecoder.cpp

 /*
  * Copyright (C) 2008-2009 Torch Mobile, Inc.
  * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Library General Public
  *  License as published by the Free Software Foundation; either
  *  version 2 of the License, or (at your option) any later version.
  *
  *  This library is distributed in the hope that it will be useful,
@@ skipping 209 matching lines @@
 int ImageDecoder::lowerBoundScaledY(int origY, int searchStart)
 {
     return getScaledValue<LowerBound>(m_scaledRows, origY, searchStart);
 }
 
 int ImageDecoder::scaledY(int origY, int searchStart)
 {
     return getScaledValue<Exact>(m_scaledRows, origY, searchStart);
 }
 
+void ImageDecoder::clearFrameBufferCache(size_t clearExceptFrame)
+{
+    // Don't clear if there is no or only one frame.
+    if (m_frameBufferCache.size() <= 1)
+        return;
+
+    // We need to preserve frames such that:
+    //  1. We don't clear |clearExceptFrame|;
+    //  2. We don't clear any frame from which a future initFrameBuffer() call
+    //     will copy bitmap data;
+    //  3. We don't clear empty frames.

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

Why not clear empty frames?  Shouldn't that just be a no-op?  It seems like we
don't need to special-case those.

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

Done.

+    // All other frames can be cleared.
+    size_t frameToBeRequired;

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

Nit: Init this to notFound; this allows eliminating the "else" below and is
safer in case someone later changes this code and accidentally adds an
uninitialized-variable codepath.

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

Done.

+    if (clearExceptFrame < m_frameBufferCache.size() && m_frameBufferCache[clearExceptFrame].status() == ImageFrame::FrameEmpty) {
+        frameToBeRequired = clearExceptFrame;
+        do {
+            frameToBeRequired = m_frameBufferCache[frameToBeRequired].requiredPreviousFrameIndex();
+        } while (frameToBeRequired != notFound && m_frameBufferCache[frameToBeRequired].status() == ImageFrame::FrameEmpty);

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

This loop tries to preserve the frame we'll need to start from if we later
decode frames >= clearExceptFrame.  But if we later ask for a frame <
clearExceptFrame, we may need to redecode the whole image to that point.

It's probably worth updating the comments on ImageSource::clear() to say that
the intent of the function is to throw away as much data as possible given an
expectation that you're next going to ask for a frame on/after the provided
frame index, and that if callers violate this by clearing and then requesting a
frame before the provided frame number, the decoder may need to redecode the
image from the beginning.

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

Done.

+    } else {
+        // initFrameBuffer() has already been called for this frame.
+        frameToBeRequired = notFound;
+    }
+
+    for (size_t i = 0; i < m_frameBufferCache.size(); ++i) {
+        if (i != clearExceptFrame && i != frameToBeRequired && m_frameBufferCache[i].status() != ImageFrame::FrameEmpty)
+            clearFrameBuffer(i);
+    }
+}
+
+void ImageDecoder::clearFrameBuffer(size_t frameIndex)
+{
+    m_frameBufferCache[frameIndex].clearPixelData();
+}
+
+size_t ImageDecoder::findRequiredPreviousFrame(size_t frameIndex)

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

By the way, the algorithm in this function can be simpler and faster if we know
that all frames before frameIndex have already had m_requiredPreviousFrameIndex
set -- because we just need to look at the previous frame's disposal method and
required previous frame, we don't have to loop through other frames.

You may want to ensure that invariant holds and then change to the simpler
algorithm.

+{
+    if (!frameIndex) {
+        // The first frame doesn't rely on any previous data.
+        return notFound;
+    }
+
+    // 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.)
+    size_t prevFrame = frameIndex - 1;
+    const ImageFrame* prevBuffer = &m_frameBufferCache[prevFrame];

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

If we're going to do this we should either check or ASSERT that frameIndex() is
<= m_frameBufferCache.size().

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

Done.

+    ImageFrame::FrameDisposalMethod prevMethod = prevBuffer->disposalMethod();
+    while (prevFrame && (prevMethod == ImageFrame::DisposeOverwritePrevious)) {
+        prevBuffer = &m_frameBufferCache[--prevFrame];
+        prevMethod = prevBuffer->disposalMethod();
+    }
+
+    if (prevMethod == ImageFrame::DisposeNotSpecified || prevMethod == ImageFrame::DisposeKeep) {
+        // prevFrame will be used as the starting state for this frame.

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

Nit: Might want to add a note or FIXME about being even smarter by checking the
frame sizes and/or alpha-containing regions.

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

Done.

+        return prevFrame;
+    }
+
+    if (!prevFrame) {
+        // The first frame whose disposal method is DisposeOverwriteBgColor or
+        // DisposeOverritePrevious will be reset to background despite its size.

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

Nit: Comment could be clearer; how about:

This frame's disposal method is DisposeOverwriteBgColor or
DisposeOverwritePrevious.  If this is the first frame, the starting state for
the next frame is guaranteed to be just an empty frame.

(I would also place this comment above the conditional instead of inside it.)

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

Done.

+        return notFound;
+    }
+
+    ASSERT(prevMethod == ImageFrame::DisposeOverwriteBgcolor);
+    const IntRect& prevRect = prevBuffer->originalFrameRect();
+    const IntSize& bufferSize = scaledSize();

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

I think scaledSize() got nuked (or is getting nuked) in a separate CL.

Also, you don't actually use this temp.

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

Done.

+    if (prevRect.contains(IntRect(IntPoint(), scaledSize()))) {
+        // prevFrame covering the whole image will be reset to background,

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

Nit: A slightly shorter way of doing this:

    // If prevRect fills the whole image, then the current frame can be decoded
    // alone.  Otherwise, prevFrame contributes to this frame.
    return prevRect.contains(IntRect(IntPoint(), size())) ? notFound :
prevFrame;

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

Done.

+        return notFound;
+    }
+
+    // prevFrame only clears part of the image to background, so it contributes
+    // to the starting state of this frame.
+    return prevFrame;
+}
+
 } // namespace WebCore