Changeset 28068 in webkit

Timestamp:

Nov 26, 2007 7:01:23 PM (16 years ago)

Author:

alp@webkit.org

Message:

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):

Location:

trunk/WebCore

Files:

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

trunk/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/WebCore/platform/image-decoders/ImageDecoder.h

@@ -42 +42 @@
 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)
     {} 
 
@@ -53 +61 @@
     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; }
 
@@ -60 +68 @@
     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; }
 
@@ -87 +95 @@
     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.
 };

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

@@ -159 +159 @@
 RGBA32Buffer* GIFImageDecoder::frameBufferAtIndex(size_t index)
 {
-    if (index < 0 || index >= frameCount())
+    if (index >= frameCount())
         return 0;
 
@@ -195 +195 @@
 }
 
-void GIFImageDecoder::initFrameBuffer(RGBA32Buffer& buffer, 
-                                      RGBA32Buffer* previousBuffer,
-                                      bool compositeWithPreviousFrame)
+void GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
 {
     // Initialize the frame rect in our buffer.
@@ -209 +207 @@
         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);
             }
         }
@@ -282 +260 @@
 
     // 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);
 }
 
@@ -291 +279 @@
                                      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.
@@ -322 +307 @@
     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];
@@ -332 +315 @@
             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++;
@@ -364 +341 @@
 }
 
-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);
+        }
+    }
 }
 

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

@@ -66 +66 @@
     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;
 };

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

@@ -642 +642 @@
           // 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;
       }
@@ -883 +885 @@
         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 */

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

@@ -77 +77 @@
 } 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 */
@@ -112 +102 @@
     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. */
@@ -141 +131 @@
         x_offset = y_offset = width = height = 0;
         tpixel = 0;
-        disposal_method = DISPOSE_NOT_SPECIFIED;
+        disposal_method = WebCore::RGBA32Buffer::DisposeNotSpecified;
 
         local_colormap = 0;