Checking if frame is complete and access duration doesn't need a decode
authorossy@webkit.org <ossy@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Jun 2013 08:44:38 +0000 (08:44 +0000)
committerossy@webkit.org <ossy@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Jun 2013 08:44:38 +0000 (08:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=116041

Reviewed by Allan Sandfeld Jensen.

This change is to avoid image decoding for these two operations:
1. frameIsCompleteAtIndex
2. frameDurationAtIndex
These two operations are moved to ImageDecoder interface and are now const
to prevent future regression.

We are now able to check if a frame is complete by parsing the entire GIF file
without decoding. This also provides information like frame duration such that
controller the animation doesn't require any decoding.

Based on the Blink patch by Hin-Chung Lam <hclam@google.com>
https://src.chromium.org/viewvc/blink?revision=149883&view=revision

* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::frameIsCompleteAtIndex):
(WebCore::BitmapImage::frameDurationAtIndex):
* platform/graphics/ImageSource.cpp:
(WebCore::ImageSource::frameDurationAtIndex):
(WebCore::ImageSource::frameHasAlphaAtIndex):
(WebCore::ImageSource::frameIsCompleteAtIndex):
* platform/graphics/ImageSource.h:
* platform/image-decoders/ImageDecoder.cpp:
(WebCore::ImageDecoder::frameHasAlphaAtIndex):
(WebCore::ImageDecoder::frameIsCompleteAtIndex):
* platform/image-decoders/ImageDecoder.h:
(WebCore::ImageDecoder::frameDurationAtIndex):
* platform/image-decoders/gif/GIFImageDecoder.cpp:
(WebCore::GIFImageDecoder::frameIsCompleteAtIndex):
(WebCore::GIFImageDecoder::frameDurationAtIndex):
(WebCore::GIFImageDecoder::haveDecodedRow):
(WebCore::GIFImageDecoder::gifComplete):
(WebCore::GIFImageDecoder::decode):
(WebCore::GIFImageDecoder::initFrameBuffer):
* platform/image-decoders/gif/GIFImageDecoder.h:
* platform/image-decoders/gif/GIFImageReader.h:
(GIFImageReader::frameContext):
(GIFImageReader::parseCompleted):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@151957 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/BitmapImage.cpp
Source/WebCore/platform/graphics/ImageSource.cpp
Source/WebCore/platform/graphics/ImageSource.h
Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp
Source/WebCore/platform/image-decoders/ImageDecoder.cpp
Source/WebCore/platform/image-decoders/ImageDecoder.h
Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp
Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h
Source/WebCore/platform/image-decoders/gif/GIFImageReader.h

index ba4d0de..6f39499 100644 (file)
@@ -1,3 +1,48 @@
+2013-06-25  Csaba Osztrogon√°c  <ossy@webkit.org>
+
+        Checking if frame is complete and access duration doesn't need a decode
+        https://bugs.webkit.org/show_bug.cgi?id=116041
+
+        Reviewed by Allan Sandfeld Jensen.
+
+        This change is to avoid image decoding for these two operations:
+        1. frameIsCompleteAtIndex
+        2. frameDurationAtIndex
+        These two operations are moved to ImageDecoder interface and are now const
+        to prevent future regression.
+
+        We are now able to check if a frame is complete by parsing the entire GIF file
+        without decoding. This also provides information like frame duration such that
+        controller the animation doesn't require any decoding.
+
+        Based on the Blink patch by Hin-Chung Lam <hclam@google.com>
+        https://src.chromium.org/viewvc/blink?revision=149883&view=revision
+
+        * platform/graphics/BitmapImage.cpp:
+        (WebCore::BitmapImage::frameIsCompleteAtIndex):
+        (WebCore::BitmapImage::frameDurationAtIndex):
+        * platform/graphics/ImageSource.cpp:
+        (WebCore::ImageSource::frameDurationAtIndex):
+        (WebCore::ImageSource::frameHasAlphaAtIndex):
+        (WebCore::ImageSource::frameIsCompleteAtIndex):
+        * platform/graphics/ImageSource.h:
+        * platform/image-decoders/ImageDecoder.cpp:
+        (WebCore::ImageDecoder::frameHasAlphaAtIndex):
+        (WebCore::ImageDecoder::frameIsCompleteAtIndex):
+        * platform/image-decoders/ImageDecoder.h:
+        (WebCore::ImageDecoder::frameDurationAtIndex):
+        * platform/image-decoders/gif/GIFImageDecoder.cpp:
+        (WebCore::GIFImageDecoder::frameIsCompleteAtIndex):
+        (WebCore::GIFImageDecoder::frameDurationAtIndex):
+        (WebCore::GIFImageDecoder::haveDecodedRow):
+        (WebCore::GIFImageDecoder::gifComplete):
+        (WebCore::GIFImageDecoder::decode):
+        (WebCore::GIFImageDecoder::initFrameBuffer):
+        * platform/image-decoders/gif/GIFImageDecoder.h:
+        * platform/image-decoders/gif/GIFImageReader.h:
+        (GIFImageReader::frameContext):
+        (GIFImageReader::parseCompleted):
+
 2013-06-24  Christophe Dumez  <ch.dumez@sisa.samsung.com>
 
         Update AbstractWorker, Worker and SharedWorker to match the specification
index 4a13d47..07610d2 100644 (file)
@@ -305,16 +305,16 @@ PassNativeImagePtr BitmapImage::frameAtIndex(size_t index)
 
 bool BitmapImage::frameIsCompleteAtIndex(size_t index)
 {
-    if (!ensureFrameIsCached(index))
-        return false;
-    return m_frames[index].m_isComplete;
+    if (index < m_frames.size() && m_frames[index].m_haveMetadata && m_frames[index].m_isComplete)
+        return true;
+    return m_source.frameIsCompleteAtIndex(index);
 }
 
 float BitmapImage::frameDurationAtIndex(size_t index)
 {
-    if (!ensureFrameIsCached(index))
-        return 0;
-    return m_frames[index].m_duration;
+    if (index < m_frames.size() && m_frames[index].m_haveMetadata)
+        return m_frames[index].m_duration;
+    return m_source.frameDurationAtIndex(index);
 }
 
 PassNativeImagePtr BitmapImage::nativeImageForCurrentFrame()
index f7b4909..25c095b 100644 (file)
@@ -155,20 +155,16 @@ PassNativeImagePtr ImageSource::createFrameAtIndex(size_t index)
     return buffer->asNewNativeImage();
 }
 
-float ImageSource::frameDurationAtIndex(size_t index)
+float ImageSource::frameDurationAtIndex(size_t index) const
 {
     if (!m_decoder)
         return 0;
 
-    ImageFrame* buffer = m_decoder->frameBufferAtIndex(index);
-    if (!buffer || buffer->status() == ImageFrame::FrameEmpty)
-        return 0;
-
     // Many annoying ads specify a 0 duration to make an image flash as quickly as possible.
     // We follow Firefox's behavior and use a duration of 100 ms for any frames that specify
     // a duration of <= 10 ms. See <rdar://problem/7689300> and <http://webkit.org/b/36082>
     // for more information.
-    const float duration = buffer->duration() / 1000.0f;
+    const float duration = m_decoder->frameDurationAtIndex(index) / 1000.0f;
     if (duration < 0.011f)
         return 0.100f;
     return duration;
@@ -179,20 +175,14 @@ ImageOrientation ImageSource::orientationAtIndex(size_t) const
     return m_decoder ? m_decoder->orientation() : DefaultImageOrientation;
 }
 
-bool ImageSource::frameHasAlphaAtIndex(size_t index)
+bool ImageSource::frameHasAlphaAtIndex(size_t index) const
 {
-    if (!m_decoder)
-        return true;
-    return m_decoder->frameHasAlphaAtIndex(index);
+    return !m_decoder || m_decoder->frameHasAlphaAtIndex(index);
 }
 
-bool ImageSource::frameIsCompleteAtIndex(size_t index)
+bool ImageSource::frameIsCompleteAtIndex(size_t index) const
 {
-    if (!m_decoder)
-        return false;
-
-    ImageFrame* buffer = m_decoder->frameBufferAtIndex(index);
-    return buffer && buffer->status() == ImageFrame::FrameComplete;
+    return m_decoder && m_decoder->frameIsCompleteAtIndex(index);
 }
 
 unsigned ImageSource::frameBytesAtIndex(size_t index) const
index 31a2e19..cd0d2b7 100644 (file)
@@ -147,9 +147,9 @@ public:
     // see comments on clear() above.
     PassNativeImagePtr createFrameAtIndex(size_t);
 
-    float frameDurationAtIndex(size_t);
-    bool frameHasAlphaAtIndex(size_t); // Whether or not the frame actually used any alpha.
-    bool frameIsCompleteAtIndex(size_t); // Whether or not the frame is completely decoded.
+    float frameDurationAtIndex(size_t) const;
+    bool frameHasAlphaAtIndex(size_t) const; // Whether or not the frame actually used any alpha.
+    bool frameIsCompleteAtIndex(size_t) const; // Whether or not the frame is completely decoded.
     ImageOrientation orientationAtIndex(size_t) const; // EXIF image orientation
 
     // Return the number of bytes in the decoded frame. If the frame is not yet
index aaa2fdf..96e21c9 100644 (file)
@@ -312,7 +312,7 @@ CGImageRef ImageSource::createFrameAtIndex(size_t index)
     return maskedImage.leakRef();
 }
 
-bool ImageSource::frameIsCompleteAtIndex(size_t index)
+bool ImageSource::frameIsCompleteAtIndex(size_t index) const
 {
     ASSERT(frameCount());
 
@@ -332,7 +332,7 @@ bool ImageSource::frameIsCompleteAtIndex(size_t index)
     return frameStatus == kCGImageStatusComplete;
 }
 
-float ImageSource::frameDurationAtIndex(size_t index)
+float ImageSource::frameDurationAtIndex(size_t index) const
 {
     if (!initialized())
         return 0;
@@ -361,7 +361,7 @@ float ImageSource::frameDurationAtIndex(size_t index)
     return duration;
 }
 
-bool ImageSource::frameHasAlphaAtIndex(size_t index)
+bool ImageSource::frameHasAlphaAtIndex(size_t index) const
 {
     if (!m_decoder)
         return false; // FIXME: why doesn't this return true?
index b9d3cb5..741a9fe 100644 (file)
@@ -267,11 +267,12 @@ template <MatchType type> int getScaledValue(const Vector<int>& scaledValues, in
 
 bool ImageDecoder::frameHasAlphaAtIndex(size_t index) const
 {
-    if (m_frameBufferCache.size() <= index)
-        return true;
-    if (m_frameBufferCache[index].status() == ImageFrame::FrameComplete)
-        return m_frameBufferCache[index].hasAlpha();
-    return true;
+    return !frameIsCompleteAtIndex(index) || m_frameBufferCache[index].hasAlpha();
+}
+
+bool ImageDecoder::frameIsCompleteAtIndex(size_t index) const
+{
+    return (index < m_frameBufferCache.size()) && (m_frameBufferCache[index].status() == ImageFrame::FrameComplete);
 }
 
 unsigned ImageDecoder::frameBytesAtIndex(size_t index) const
index fc337af..fab7652 100644 (file)
@@ -274,6 +274,12 @@ namespace WebCore {
         // Make the best effort guess to check if the requested frame has alpha channel.
         virtual bool frameHasAlphaAtIndex(size_t) const;
 
+        // Whether or not the frame is fully received.
+        virtual bool frameIsCompleteAtIndex(size_t) const;
+
+        // Duration for displaying a frame in seconds. This method is used by animated images only.
+        virtual float frameDurationAtIndex(size_t) const { return 0; }
+
         // Number of bytes in the decoded frame requested. Return 0 if not yet decoded.
         virtual unsigned frameBytesAtIndex(size_t) const;
 
index 2aa6c66..cd32df4 100644 (file)
@@ -127,6 +127,19 @@ ImageFrame* GIFImageDecoder::frameBufferAtIndex(size_t index)
     return &frame;
 }
 
+
+bool GIFImageDecoder::frameIsCompleteAtIndex(size_t index) const
+{
+    return m_reader && (index < m_reader->imagesCount()) && m_reader->frameContext(index)->isComplete();
+}
+
+float GIFImageDecoder::frameDurationAtIndex(size_t index) const
+{
+    return (m_reader && (index < m_reader->imagesCount()) && m_reader->frameContext(index)->isHeaderDefined())
+        ? m_reader->frameContext(index)->delayTime : 0;
+}
+
+
 bool GIFImageDecoder::setFailed()
 {
     m_reader.clear();
@@ -185,7 +198,7 @@ void GIFImageDecoder::clearFrameBufferCache(size_t clearBeforeFrame)
 
 bool GIFImageDecoder::haveDecodedRow(unsigned frameIndex, const Vector<unsigned char>& rowBuffer, size_t width, size_t rowNumber, unsigned repeatCount, bool writeTransparentPixels)
 {
-    const GIFFrameContext* frameContext = m_reader->frameContext();
+    const GIFFrameContext* frameContext = m_reader->frameContext(frameIndex);
     // The pixel data and coordinates supplied to us are relative to the frame's
     // origin within the entire image size, i.e.
     // (frameContext->xOffset, frameContext->yOffset). There is no guarantee
@@ -297,8 +310,6 @@ void GIFImageDecoder::gifComplete()
     // Cache the repetition count, which is now as authoritative as it's ever
     // going to be.
     repetitionCount();
-
-    m_reader.clear();
 }
 
 void GIFImageDecoder::decode(unsigned haltAtFrame, GIFQuery query)
@@ -335,16 +346,16 @@ void GIFImageDecoder::decode(unsigned haltAtFrame, GIFQuery query)
         return;
     }
 
-    // It is also a fatal error if all data is received but we failed to decode
-    // all frames completely.
-    if (isAllDataReceived() && haltAtFrame >= m_frameBufferCache.size() && m_reader)
+    // 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())
         setFailed();
 }
 
 bool GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
 {
     // Initialize the frame rect in our buffer.
-    const GIFFrameContext* frameContext = m_reader->frameContext();
+    const GIFFrameContext* frameContext = m_reader->frameContext(frameIndex);
     IntRect frameRect(frameContext->xOffset, frameContext->yOffset, frameContext->width, frameContext->height);
 
     // Make sure the frameRect doesn't extend outside the buffer.
index f9e1b84..b1e95fa 100644 (file)
@@ -49,6 +49,8 @@ namespace WebCore {
         virtual size_t frameCount();
         virtual int repetitionCount() const;
         virtual ImageFrame* frameBufferAtIndex(size_t index);
+        virtual bool frameIsCompleteAtIndex(size_t) const;
+        virtual float frameDurationAtIndex(size_t) const;
         // CAUTION: setFailed() deletes |m_reader|.  Be careful to avoid
         // accessing deleted memory, especially when calling this from inside
         // GIFImageReader!
index bda8997..b2cefbe 100644 (file)
@@ -282,11 +282,13 @@ public:
         return frame->isLocalColormapDefined ? frame->localColormapSize : 0;
     }
 
-    const GIFFrameContext* frameContext() const
+    const GIFFrameContext* frameContext(size_t index) const
     {
-        return m_currentDecodingFrame < m_frames.size() ? m_frames[m_currentDecodingFrame].get() : 0;
+        return index < m_frames.size() ? m_frames[index].get() : 0;
     }
 
+    bool parseCompleted() const { return m_parseCompleted; }
+
 private:
     bool parse(size_t dataPosition, size_t len, bool parseSizeOnly);
     void setRemainingBytes(size_t);