Changeset 222910 in webkit

Timestamp:

Oct 5, 2017 7:00:05 AM (7 years ago)

Author:

magomez@igalia.com

Message:

[GTK][WPE] GIFImageDecoder never clears decoded frames even when told to do so
​https://bugs.webkit.org/show_bug.cgi?id=177864

Reviewed by Carlos Garcia Campos.

Fix GIFImageDecoder::clearFrameBufferCache() so it really deletes decoded buffers, and modify
GIFImageDecoder to be able to decode frames that are not requested in the expected order.

Covered by existent tests.

• platform/image-decoders/gif/GIFImageDecoder.cpp:

(WebCore::GIFImageDecoder::findFirstRequiredFrameToDecode):
(WebCore::GIFImageDecoder::frameBufferAtIndex):
(WebCore::GIFImageDecoder::clearFrameBufferCache):

• platform/image-decoders/gif/GIFImageDecoder.h:
• platform/image-decoders/gif/GIFImageReader.cpp:

(GIFImageReader::decode):

• platform/image-decoders/gif/GIFImageReader.h:

(GIFImageReader::frameContext const):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/image-decoders/gif/GIFImageDecoder.cpp (modified) (3 diffs)
• platform/image-decoders/gif/GIFImageDecoder.h (modified) (1 diff)
• platform/image-decoders/gif/GIFImageReader.cpp (modified) (1 diff)
• platform/image-decoders/gif/GIFImageReader.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-10-05  Miguel Gomez  <magomez@igalia.com>
+
+        [GTK][WPE] GIFImageDecoder never clears decoded frames even when told to do so
+        https://bugs.webkit.org/show_bug.cgi?id=177864
+
+        Reviewed by Carlos Garcia Campos.
+
+        Fix GIFImageDecoder::clearFrameBufferCache() so it really deletes decoded buffers, and modify
+        GIFImageDecoder to be able to decode frames that are not requested in the expected order.
+
+        Covered by existent tests.
+
+        * platform/image-decoders/gif/GIFImageDecoder.cpp:
+        (WebCore::GIFImageDecoder::findFirstRequiredFrameToDecode):
+        (WebCore::GIFImageDecoder::frameBufferAtIndex):
+        (WebCore::GIFImageDecoder::clearFrameBufferCache):
+        * platform/image-decoders/gif/GIFImageDecoder.h:
+        * platform/image-decoders/gif/GIFImageReader.cpp:
+        (GIFImageReader::decode):
+        * platform/image-decoders/gif/GIFImageReader.h:
+        (GIFImageReader::frameContext const):
+
 2017-10-05  Fujii Hironori  <Hironori.Fujii@sony.com>
 

trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp

@@ -102 +102 @@
 }
 
+size_t GIFImageDecoder::findFirstRequiredFrameToDecode(size_t frameIndex)
+{
+    // The first frame doesn't depend on any other.
+    if (!frameIndex)
+        return 0;
+
+    for (size_t i = frameIndex; i > 0; --i) {
+        ImageFrame& frame = m_frameBufferCache[i - 1];
+
+        // Frames with disposal method RestoreToPrevious are useless, skip them.
+        if (frame.disposalMethod() == ImageFrame::DisposalMethod::RestoreToPrevious)
+            continue;
+
+        // At this point the disposal method can be Unspecified, DoNotDispose or RestoreToBackground.
+        // In every case, if the frame is complete we can start decoding the next one.
+        if (frame.isComplete())
+            return i;
+
+        // If the disposal method of this frame is RestoreToBackground and it fills the whole area,
+        // the next frame's backing store is initialized to transparent, so we start decoding with it.
+        if (frame.disposalMethod() == ImageFrame::DisposalMethod::RestoreToBackground) {
+            // We cannot use frame.backingStore()->frameRect() here, because it has been cleared
+            // when the frame was removed from the cache. We need to get the values from the
+            // reader context.
+            const auto* frameContext = m_reader->frameContext(i - 1);
+            ASSERT(frameContext);
+            IntRect frameRect(frameContext->xOffset, frameContext->yOffset, frameContext->width, frameContext->height);
+            // We would need to scale frameRect and check whether it fills the whole scaledSize(). But
+            // can check whether the original frameRect fills size() instead. If the frame fills the
+            // whole area then it can be decoded without dependencies.
+            if (frameRect.contains({ { }, size() }))
+                return i;
+        }
+    }
+
+    return 0;
+}
+
 ImageFrame* GIFImageDecoder::frameBufferAtIndex(size_t index)
 {
@@ -108 +146 @@
 
     ImageFrame& frame = m_frameBufferCache[index];
-    if (!frame.isComplete())
-        decode(index + 1, GIFFullQuery, isAllDataReceived());
+    if (!frame.isComplete()) {
+        for (auto i = findFirstRequiredFrameToDecode(index); i <= index; i++)
+            decode(i + 1, GIFFullQuery, isAllDataReceived());
+    }
+
     return &frame;
 }
@@ -164 +205 @@
     for (Vector<ImageFrame>::iterator j(m_frameBufferCache.begin()); j != i; ++j) {
         ASSERT(!j->isPartial());
-        if (j->isInvalid())
+        if (!j->isInvalid())
             j->clear();
     }

trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h

@@ -66 +66 @@
     GIFImageDecoder(AlphaOption, GammaAndColorProfileOption);
     void tryDecodeSize(bool allDataReceived) final { decode(0, GIFSizeQuery, allDataReceived); }
+    size_t findFirstRequiredFrameToDecode(size_t);
 
     // If the query is GIFFullQuery, decodes the image up to (but not

trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp

@@ -366 +366 @@
     // Already decoded frames can be deleted from the cache and then they require to be decoded again, so
     // the haltAtFrame value we receive here may be lower than m_currentDecodingFrame. In this case
-    // we position m_currentDecodingFrame to haltAtFrame and decode from there.
+    // we position m_currentDecodingFrame to haltAtFrame - 1 and decode from there.
     // See bug https://bugs.webkit.org/show_bug.cgi?id=176089.
-    m_currentDecodingFrame = std::min(m_currentDecodingFrame, static_cast<size_t>(haltAtFrame));
+    m_currentDecodingFrame = std::min(m_currentDecodingFrame, static_cast<size_t>(haltAtFrame) - 1);
 
     while (m_currentDecodingFrame < std::min(m_frames.size(), static_cast<size_t>(haltAtFrame))) {

trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.h

@@ -285 +285 @@
     }
 
+    const GIFFrameContext* frameContext(size_t frame) const
+    {
+        return frame < m_frames.size() ? m_frames[frame].get() : nullptr;
+    }
+
 private:
     bool parse(size_t dataPosition, size_t len, bool parseSizeOnly);