2007-11-26 Peter Kasting <zerodpx@gmail.com>
authoralp@webkit.org <alp@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Nov 2007 03:01:23 +0000 (03:01 +0000)
committeralp@webkit.org <alp@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Nov 2007 03:01:23 +0000 (03:01 +0000)
        Reviewed by Alp Toker.

        http://bugs.webkit.org/show_bug.cgi?id=15974
        GIF decoding should respect frames' specified disposal methods.

        * platform/image-decoders/ImageDecoder.h:
        (WebCore::RGBA32Buffer::):
        (WebCore::RGBA32Buffer::RGBA32Buffer):
        (WebCore::RGBA32Buffer::disposalMethod):
        (WebCore::RGBA32Buffer::setDisposalMethod):
        * platform/image-decoders/gif/GIFImageDecoder.cpp:
        (WebCore::GIFImageDecoder::frameBufferAtIndex):
        (WebCore::GIFImageDecoder::initFrameBuffer):
        (WebCore::GIFImageDecoder::prepEmptyFrameBuffer):
        (WebCore::GIFImageDecoder::haveDecodedRow):
        (WebCore::GIFImageDecoder::frameComplete):
        * platform/image-decoders/gif/GIFImageDecoder.h:
        * platform/image-decoders/gif/GIFImageReader.cpp:
        (GIFImageReader::read):
        * platform/image-decoders/gif/GIFImageReader.h:
        (GIFFrameReader::GIFFrameReader):

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

WebCore/ChangeLog
WebCore/platform/image-decoders/ImageDecoder.h
WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp
WebCore/platform/image-decoders/gif/GIFImageDecoder.h
WebCore/platform/image-decoders/gif/GIFImageReader.cpp
WebCore/platform/image-decoders/gif/GIFImageReader.h

index 3a0f95be3b33bb10e71211736b6c9331ca2283cc..54febc491936cac6a0334d5def6ba9be9f6df5f4 100644 (file)
@@ -1,3 +1,27 @@
+2007-11-26  Peter Kasting  <zerodpx@gmail.com>
+
+        Reviewed by Alp Toker.
+
+        http://bugs.webkit.org/show_bug.cgi?id=15974
+        GIF decoding should respect frames' specified disposal methods.
+
+        * platform/image-decoders/ImageDecoder.h:
+        (WebCore::RGBA32Buffer::):
+        (WebCore::RGBA32Buffer::RGBA32Buffer):
+        (WebCore::RGBA32Buffer::disposalMethod):
+        (WebCore::RGBA32Buffer::setDisposalMethod):
+        * platform/image-decoders/gif/GIFImageDecoder.cpp:
+        (WebCore::GIFImageDecoder::frameBufferAtIndex):
+        (WebCore::GIFImageDecoder::initFrameBuffer):
+        (WebCore::GIFImageDecoder::prepEmptyFrameBuffer):
+        (WebCore::GIFImageDecoder::haveDecodedRow):
+        (WebCore::GIFImageDecoder::frameComplete):
+        * platform/image-decoders/gif/GIFImageDecoder.h:
+        * platform/image-decoders/gif/GIFImageReader.cpp:
+        (GIFImageReader::read):
+        * platform/image-decoders/gif/GIFImageReader.h:
+        (GIFFrameReader::GIFFrameReader):
+
 2007-11-26  Adam Roben  <aroben@apple.com>
 
         Add a Color(CGColorRef) constructor
index ed1d0542a05dbb5253b7fca24f8e9b0cc21cf67a..3df2e880a2a0b124aa7a03f6a9c75cd9b58f7621 100644 (file)
@@ -41,9 +41,17 @@ class RGBA32Buffer
 {
 public:
     enum FrameStatus { FrameEmpty, FramePartial, FrameComplete };
+    enum FrameDisposalMethod {
+        // If you change the numeric values of these, make sure you audit all
+        // users, as some users may cast raw values to/from these constants.
+        DisposeNotSpecified = 0,       // Leave frame in framebuffer
+        DisposeKeep = 1,               // Leave frame in framebuffer
+        DisposeOverwriteBgcolor = 2,   // Clear frame to transparent
+        DisposeOverwritePrevious = 3,  // Clear frame to previous framebuffer contents
+    };
 
     RGBA32Buffer() : m_height(0), m_status(FrameEmpty), m_duration(0),
-                     m_includeInNextFrame(false), m_hasAlpha(false)
+                     m_disposalMethod(DisposeNotSpecified), m_hasAlpha(false)
     {} 
 
     const RGBA32Array& bytes() const { return m_bytes; }
@@ -52,14 +60,14 @@ public:
     unsigned height() const { return m_height; }
     FrameStatus status() const { return m_status; }
     unsigned duration() const { return m_duration; }
-    bool includeInNextFrame() const { return m_includeInNextFrame; }
+    FrameDisposalMethod disposalMethod() const { return m_disposalMethod; }
     bool hasAlpha() const { return m_hasAlpha; }
 
     void setRect(const IntRect& r) { m_rect = r; }
     void ensureHeight(unsigned rowIndex) { if (rowIndex > m_height) m_height = rowIndex; }
     void setStatus(FrameStatus s) { m_status = s; }
     void setDuration(unsigned duration) { m_duration = duration; }
-    void setIncludeInNextFrame(bool n) { m_includeInNextFrame = n; }
+    void setDisposalMethod(FrameDisposalMethod method) { m_disposalMethod = method; }
     void setHasAlpha(bool alpha) { m_hasAlpha = alpha; }
 
     static void setRGBA(unsigned& pos, unsigned r, unsigned g, unsigned b, unsigned a)
@@ -86,7 +94,7 @@ private:
     unsigned m_height; // The height (the number of rows we've fully decoded).
     FrameStatus m_status; // Whether or not this frame is completely finished decoding.
     unsigned m_duration; // The animation delay.
-    bool m_includeInNextFrame; // Whether or not the next buffer should be initially populated with our data.
+    FrameDisposalMethod m_disposalMethod; // What to do with this frame's data when initializing the next frame.
     bool m_hasAlpha; // Whether or not any of the pixels in the buffer have transparency.
 };
 
index b7ea6b2de2615c7fbbf11389540992f6e7b13162..c0b93b5895be71d7739b13bc359560faa00607c8 100644 (file)
@@ -158,7 +158,7 @@ int GIFImageDecoder::repetitionCount() const
 
 RGBA32Buffer* GIFImageDecoder::frameBufferAtIndex(size_t index)
 {
-    if (index < 0 || index >= frameCount())
+    if (index >= frameCount())
         return 0;
 
     RGBA32Buffer& frame = m_frameBufferCache[index];
@@ -194,9 +194,7 @@ void GIFImageDecoder::decodingHalted(unsigned bytesLeft)
     m_reader->setReadOffset(m_data->size() - bytesLeft);
 }
 
-void GIFImageDecoder::initFrameBuffer(RGBA32Buffer& buffer, 
-                                      RGBA32Buffer* previousBuffer,
-                                      bool compositeWithPreviousFrame)
+void GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
 {
     // Initialize the frame rect in our buffer.
     IntRect frameRect(m_reader->frameXOffset(), m_reader->frameYOffset(),
@@ -208,80 +206,70 @@ void GIFImageDecoder::initFrameBuffer(RGBA32Buffer& buffer,
     if (frameRect.bottom() > m_size.height())
         frameRect.setHeight(m_size.height() - m_reader->frameYOffset());
 
-    buffer.setRect(frameRect);
-
-    bool isSubRect = (frameRect.x() > 0 || frameRect.y() > 0 ||
-                      frameRect.width() < m_size.width() ||
-                      frameRect.height() < m_size.height());
+    RGBA32Buffer* const buffer = &m_frameBufferCache[frameIndex];
+    buffer->setRect(frameRect);
     
-    // Let's resize our buffer now to the correct width/height and then
-    // initialize portions of it if needed.
-    RGBA32Array& bytes = buffer.bytes();
-        
-    // If the disposal method of the previous frame said to stick around, then we need     
-    // to copy that frame into our frame.  We also dont want to have any impact on
-    // anything outside our frame's rect, so if we don't overlay the entire image,
-    // then also composite with the previous frame.
-    if (previousBuffer && (compositeWithPreviousFrame || isSubRect)) {
-        bytes = previousBuffer->bytes();
-        buffer.ensureHeight(m_size.height());
-        buffer.setHasAlpha(previousBuffer->hasAlpha());
-    }
-    else // Resize to the width and height of the image.
-        bytes.resize(m_size.width() * m_size.height());
-
-    if (isSubRect) {
-        // We need to go ahead and initialize the first frame to make sure
-        // that areas outside the subrect start off transparent.
-        if (!previousBuffer) {
-            bytes.fill(0);
-            buffer.setHasAlpha(true);
-        } else if (!compositeWithPreviousFrame) {
-            // Now this is an interesting case.  In the case where we fill 
-            // the entire image, we effectively do a full clear of the image (and thus
-            // don't have to initialize anything in our buffer).
-            // 
-            // However in the case where we only fill a piece of the image, two problems occur:
-            // (1) We need to wipe out the area occupied by the previous frame, which
-            // could also have been a subrect.
-            // (2) Anything outside the previous frame's rect *and* outside our current
-            // frame's rect should be left alone.
-            // We have handled (2) by just initializing our buffer from the previous frame.
-            // Our subrect will correctly overwrite the previous frame's contents as we
-            // decode rows.  However that still leaves the problem of having to wipe out
-            // the area occupied by the previous frame that does not overlap with
-            // the new frame.
-            if (previousBuffer->rect() != frameRect) {
-                // We have to restore the entire previous subframe with the first frame's contents.
-                RGBA32Buffer* firstBuffer = &m_frameBufferCache[0];
-                bool sawAlpha = buffer.hasAlpha();
-                IntRect prevRect = previousBuffer->rect();
-                unsigned end = prevRect.y() + prevRect.height();
-
-                // Given that we allocate buffers to be the same size as previous buffers,
-                // I think this assert should be valid.
-                ASSERT(IntRect(IntPoint(0,0), m_size).contains(firstBuffer->rect()));
-
-                for (unsigned i = prevRect.y(); i < end; i++) {
-                    unsigned* curr = buffer.bytes().data() + (i * m_size.width() + prevRect.x());
-                    unsigned* orig = firstBuffer->bytes().data() + (i * m_size.width() + prevRect.x());
-                    unsigned* end = curr + prevRect.width();
-                    unsigned* origEnd = orig + firstBuffer->rect().width();
-
-                    while (curr != end && orig != origEnd) {
-                        if (!sawAlpha) {
-                            sawAlpha = true;
-                            buffer.setHasAlpha(true);
-                        }
-                        *curr++ = *orig++;
-                    }
-                }
+    if (frameIndex == 0) {
+        // This is the first frame, so we're not relying on any previous data.
+        prepEmptyFrameBuffer(buffer);
+    } 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 RGBA32Buffer* prevBuffer = &m_frameBufferCache[--frameIndex];
+        RGBA32Buffer::FrameDisposalMethod prevMethod =
+            prevBuffer->disposalMethod();
+        while ((frameIndex > 0) &&
+                (prevMethod == RGBA32Buffer::DisposeOverwritePrevious)) {
+            prevBuffer = &m_frameBufferCache[--frameIndex];
+            prevMethod = prevBuffer->disposalMethod();
+        }
+
+        if ((prevMethod == RGBA32Buffer::DisposeNotSpecified) ||
+                (prevMethod == RGBA32Buffer::DisposeKeep)) {
+            // Preserve the last frame as the starting state for this frame.
+            buffer->bytes() = prevBuffer->bytes();
+        } else {
+            // We want to clear the previous frame to transparent, without
+            // affecting pixels in the image outside of the frame.
+            const IntRect& prevRect = prevBuffer->rect();
+            if ((frameIndex == 0) ||
+                    prevRect.contains(IntRect(IntPoint(0, 0), m_size))) {
+                // Clearing the first frame, or a frame the size of the whole
+                // image, results in a completely empty image.
+                prepEmptyFrameBuffer(buffer);
+            } else {
+              // Copy the whole previous buffer, then clear just its frame.
+              buffer->bytes() = prevBuffer->bytes();
+              for (int y = prevRect.y(); y < prevRect.bottom(); ++y) {
+                  unsigned* const currentRow =
+                      buffer->bytes().data() + (y * m_size.width());
+                  for (int x = prevRect.x(); x < prevRect.right(); ++x)
+                      buffer->setRGBA(*(currentRow + x), 0, 0, 0, 0);
+              }
+              if ((prevRect.width() > 0) && (prevRect.height() > 0))
+                buffer->setHasAlpha(true);
             }
         }
     }
 
     // Update our status to be partially complete.
-    buffer.setStatus(RGBA32Buffer::FramePartial);
+    buffer->setStatus(RGBA32Buffer::FramePartial);
+
+    // Reset the alpha pixel tracker for this frame.
+    m_currentBufferSawAlpha = false;
+}
+
+void GIFImageDecoder::prepEmptyFrameBuffer(RGBA32Buffer* buffer) const
+{
+    buffer->bytes().resize(m_size.width() * m_size.height());
+    buffer->bytes().fill(0);
+    buffer->setHasAlpha(true);
 }
 
 void GIFImageDecoder::haveDecodedRow(unsigned frameIndex,
@@ -290,13 +278,10 @@ void GIFImageDecoder::haveDecodedRow(unsigned frameIndex,
                                      unsigned rowNumber,  // The row index
                                      unsigned repeatCount) // How many times to repeat the row
 {
-    // Resize to the width and height of the image.
+    // Initialize the frame if necessary.
     RGBA32Buffer& buffer = m_frameBufferCache[frameIndex];
-    RGBA32Buffer* previousBuffer = (frameIndex > 0) ? &m_frameBufferCache[frameIndex-1] : 0;
-    bool compositeWithPreviousFrame = previousBuffer && previousBuffer->includeInNextFrame();
-
     if (buffer.status() == RGBA32Buffer::FrameEmpty)
-        initFrameBuffer(buffer, previousBuffer, compositeWithPreviousFrame);
+        initFrameBuffer(frameIndex);
 
     // Do nothing for bogus data.
     if (rowBuffer == 0 || static_cast<int>(m_reader->frameYOffset() + rowNumber) >= m_size.height())
@@ -321,23 +306,15 @@ void GIFImageDecoder::haveDecodedRow(unsigned frameIndex,
     unsigned* currDst = dst;
     unsigned char* currentRowByte = rowBuffer;
     
-    bool hasAlpha = m_reader->isTransparent(); 
-    bool sawAlpha = false;
     while (currentRowByte != rowEnd && currDst < dstEnd) {
-        if ((!hasAlpha || *currentRowByte != m_reader->transparentPixel()) && *currentRowByte < colorMapSize) {
+        if ((!m_reader->isTransparent() || *currentRowByte != m_reader->transparentPixel()) && *currentRowByte < colorMapSize) {
             unsigned colorIndex = *currentRowByte * 3;
             unsigned red = colorMap[colorIndex];
             unsigned green = colorMap[colorIndex + 1];
             unsigned blue = colorMap[colorIndex + 2];
             RGBA32Buffer::setRGBA(*currDst, red, green, blue, 255);
         } else {
-            if (!sawAlpha) {
-                sawAlpha = true;
-                buffer.setHasAlpha(true);
-            }
-            
-            if (!compositeWithPreviousFrame)
-                RGBA32Buffer::setRGBA(*currDst, 0, 0, 0, 0);
+            m_currentBufferSawAlpha = true;
         }
         currDst++;
         currentRowByte++;
@@ -363,13 +340,49 @@ void GIFImageDecoder::haveDecodedRow(unsigned frameIndex,
     buffer.ensureHeight(rowNumber + repeatCount);
 }
 
-void GIFImageDecoder::frameComplete(unsigned frameIndex, unsigned frameDuration, bool includeInNextFrame)
+void GIFImageDecoder::frameComplete(unsigned frameIndex, unsigned frameDuration, RGBA32Buffer::FrameDisposalMethod disposalMethod)
 {
     RGBA32Buffer& buffer = m_frameBufferCache[frameIndex];
+    buffer.ensureHeight(m_size.height());
     buffer.setStatus(RGBA32Buffer::FrameComplete);
     buffer.setDuration(frameDuration);
-    buffer.setIncludeInNextFrame(includeInNextFrame);
-    buffer.ensureHeight(m_size.height());
+    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.rect().contains(IntRect(IntPoint(0, 0), m_size))) {
+            buffer.setHasAlpha(false);
+        } else if (frameIndex > 0) {
+            // 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 RGBA32Buffer* prevBuffer = &m_frameBufferCache[--frameIndex];
+            while ((frameIndex > 0) &&
+                    (prevBuffer->disposalMethod() ==
+                        RGBA32Buffer::DisposeOverwritePrevious))
+                prevBuffer = &m_frameBufferCache[--frameIndex];
+
+            // 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
+            // since in initFrameBuffer() we already copied that frame's alpha
+            // state into the current frame's, we need do nothing at all here.
+            //
+            // The only remaining case is a DisposeOverwriteBgcolor frame.  If
+            // it had no alpha, and its rect is contained in the current frame's
+            // rect, we know the current frame has no alpha.
+            if ((prevBuffer->disposalMethod() ==
+                    RGBA32Buffer::DisposeOverwriteBgcolor) &&
+                    !prevBuffer->hasAlpha() &&
+                    buffer.rect().contains(prevBuffer->rect()))
+                buffer.setHasAlpha(false);
+        }
+    }
 }
 
 void GIFImageDecoder::gifComplete()
index 60d63e67b56879d5b501ca29ea9fff441af8c281..e2fc76717fa802895dbc6fdb8af5877cbe0b22cc 100644 (file)
@@ -65,18 +65,20 @@ public:
     void decodingHalted(unsigned bytesLeft);
     void haveDecodedRow(unsigned frameIndex, unsigned char* rowBuffer, unsigned char* rowEnd, unsigned rowNumber, 
                         unsigned repeatCount);
-    void frameComplete(unsigned frameIndex, unsigned frameDuration, bool includeInNextFrame);
+    void frameComplete(unsigned frameIndex, unsigned frameDuration, RGBA32Buffer::FrameDisposalMethod disposalMethod);
     void gifComplete();
 
 private:
-    // Called to initialize a new frame buffer (potentially compositing it
-    // with the previous frame and/or clearing bits in our image based off
-    // the previous frame as well).
-    void initFrameBuffer(RGBA32Buffer& buffer,
-                         RGBA32Buffer* previousBuffer,
-                         bool compositeWithPreviousFrame);
+    // Called to initialize the frame buffer with the given index, based on the
+    // previous frame's disposal method.
+    void initFrameBuffer(unsigned frameIndex);
+
+    // A helper for initFrameBuffer(), this sets the size of the buffer, and
+    // fills it with transparent pixels.
+    void prepEmptyFrameBuffer(RGBA32Buffer* buffer) const;
 
     bool m_frameCountValid;
+    bool m_currentBufferSawAlpha;
     mutable GIFImageDecoderPrivate* m_reader;
 };
 
index b49915100936dc8154e3e83c5f67a2e3a10b3414..0f9b2e9bb0080a310706a367800e5580b00bf7d2 100644 (file)
@@ -641,11 +641,13 @@ bool GIFImageReader::read(const unsigned char *buf, unsigned len,
           frame_reader->is_transparent = false;
           // ignoring gfx control extension
         }
-        frame_reader->disposal_method = (gdispose)(((*q) >> 2) & 0x7);
+        // NOTE: This relies on the values in the FrameDisposalMethod enum
+        // matching those in the GIF spec!
+        frame_reader->disposal_method = (WebCore::RGBA32Buffer::FrameDisposalMethod)(((*q) >> 2) & 0x7);
         // Some specs say 3rd bit (value 4), other specs say value 3
         // Let's choose 3 (the more popular)
         if (frame_reader->disposal_method == 4)
-          frame_reader->disposal_method = (gdispose)3;
+          frame_reader->disposal_method = WebCore::RGBA32Buffer::DisposeOverwritePrevious;
         frame_reader->delay_time = GETINT16(q + 1) * 10;
       }
       GETN(1, gif_consume_block);
@@ -882,7 +884,7 @@ bool GIFImageReader::read(const unsigned char *buf, unsigned len,
         // CALLBACK: The frame is now complete.
         if (clientptr && frame_reader)
           clientptr->frameComplete(images_decoded - 1, frame_reader->delay_time, 
-                                   frame_reader->disposal_method == DISPOSE_KEEP);
+                                   frame_reader->disposal_method);
 
         /* Clear state from this image */
         if (frame_reader) {
index c0c543b03907812c13c2de7d72021a59ada0b927..faa08d27459d1ab16d9c13aadf0ede79ab3e4ef3 100644 (file)
@@ -76,16 +76,6 @@ typedef enum {
     gif_consume_comment
 } gstate;
 
-/* "Disposal" method indicates how the image should be handled in the
-   framebuffer before the subsequent image is displayed. */
-typedef enum 
-{
-    DISPOSE_NOT_SPECIFIED      = 0,
-    DISPOSE_KEEP               = 1, /* Leave it in the framebuffer */
-    DISPOSE_OVERWRITE_BGCOLOR  = 2, /* Overwrite with background color */
-    DISPOSE_OVERWRITE_PREVIOUS = 3  /* Save-under */
-} gdispose;
-
 struct GIFFrameReader {
     /* LZW decoder state machine */
     unsigned char *stackp;              /* Current stack pointer */
@@ -111,7 +101,7 @@ struct GIFFrameReader {
     unsigned int x_offset, y_offset;    /* With respect to "screen" origin */
     unsigned int height, width;
     int tpixel;                 /* Index of transparent pixel */
-    gdispose disposal_method;   /* Restore to background, leave in place, etc.*/
+    WebCore::RGBA32Buffer::FrameDisposalMethod disposal_method;   /* Restore to background, leave in place, etc.*/
     unsigned char *local_colormap;    /* Per-image colormap */
     int local_colormap_size;    /* Size of local colormap array. */
     
@@ -140,7 +130,7 @@ struct GIFFrameReader {
 
         x_offset = y_offset = width = height = 0;
         tpixel = 0;
-        disposal_method = DISPOSE_NOT_SPECIFIED;
+        disposal_method = WebCore::RGBA32Buffer::DisposeNotSpecified;
 
         local_colormap = 0;
         local_colormap_size = 0;