2007-11-28 Peter Kasting <zerodpx@gmail.com>
authoralp@webkit.org <alp@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Nov 2007 19:57:31 +0000 (19:57 +0000)
committeralp@webkit.org <alp@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Nov 2007 19:57:31 +0000 (19:57 +0000)
        Reviewed by Alp Toker.

        http://bugs.webkit.org/show_bug.cgi?id=16169
        GIF decoder needs to set hasAlpha() correctly on subsequent frames.

        This also removes the workaround for this problem in
        ImageSourceCairo.cpp.

        * platform/graphics/cairo/ImageSourceCairo.cpp:
        (WebCore::ImageSource::frameHasAlphaAtIndex):
        * platform/image-decoders/gif/GIFImageDecoder.cpp:
        (WebCore::GIFImageDecoder::initFrameBuffer):

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

WebCore/ChangeLog
WebCore/platform/graphics/cairo/ImageSourceCairo.cpp
WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp

index d995eebc86ccfdd89ebff8ea7d205f5ceed00e89..08dbb5008769afdb30ecbd64ba009128f7a725eb 100644 (file)
@@ -1,3 +1,18 @@
+2007-11-28  Peter Kasting  <zerodpx@gmail.com>
+
+        Reviewed by Alp Toker.
+
+        http://bugs.webkit.org/show_bug.cgi?id=16169
+        GIF decoder needs to set hasAlpha() correctly on subsequent frames.
+        
+        This also removes the workaround for this problem in
+        ImageSourceCairo.cpp.
+
+        * platform/graphics/cairo/ImageSourceCairo.cpp:
+        (WebCore::ImageSource::frameHasAlphaAtIndex):
+        * platform/image-decoders/gif/GIFImageDecoder.cpp:
+        (WebCore::GIFImageDecoder::initFrameBuffer):
+
 2007-11-28  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Darin Adler and Adam Roben.
index fd591662746c31940471b3945ea91ed458bd5d00..7d0b4bedf7baba319ae56b58f23ec6f1f44bdb49 100644 (file)
@@ -193,36 +193,15 @@ float ImageSource::frameDurationAtIndex(size_t index)
 
 bool ImageSource::frameHasAlphaAtIndex(size_t index)
 {
-    // We almost always want to support alpha, especially for images that are
-    // only partially loaded.
-    //
-    // There is one exception:
-    //
-    // As an optimization we can allow the image renderer to blit the image
-    // (implemented as CompositeCopy in ImageCairo) by returning false if and
-    // only if:
-    //
-    //   * The image has been fully loaded
-    //   * The buffer is marked as not having alpha transparency
-
+    // When a frame has not finished decoding, always mark it as having alpha,
+    // so we don't get a black background for the undecoded sections.
+    // TODO: A better solution is probably to have the underlying buffer's
+    // hasAlpha() return true in these cases, since it is, in fact, technically
+    // true.
     if (!frameIsCompleteAtIndex(index))
         return true;
 
-    ASSERT(m_decoder);
-    RGBA32Buffer* buffer = m_decoder->frameBufferAtIndex(index);
-    ASSERT(buffer);
-
-    // FIXME: This is a hack that makes the whole optimization useless in
-    // many cases. It is necessary because buffer->hasAlpha() incorrectly
-    // returns false when it should return true for certain GIF images.
-    //
-    // We should ideally never have to check if the decoder supports alpha.
-    //
-    // See http://bugs.webkit.org/show_bug.cgi?id=16169
-    if (m_decoder->supportsAlpha())
-        return true;
-
-    return buffer->hasAlpha();
+    return m_decoder->frameBufferAtIndex(index)->hasAlpha();
 }
 
 }
index c0b93b5895be71d7739b13bc359560faa00607c8..fa8d46d978d1258ab44e5e37fdccb3914fd75840 100644 (file)
@@ -234,6 +234,7 @@ void GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
                 (prevMethod == RGBA32Buffer::DisposeKeep)) {
             // Preserve the last frame as the starting state for this frame.
             buffer->bytes() = prevBuffer->bytes();
+            buffer->setHasAlpha(prevBuffer->hasAlpha());
         } else {
             // We want to clear the previous frame to transparent, without
             // affecting pixels in the image outside of the frame.
@@ -246,6 +247,7 @@ void GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
             } else {
               // Copy the whole previous buffer, then clear just its frame.
               buffer->bytes() = prevBuffer->bytes();
+              buffer->setHasAlpha(prevBuffer->hasAlpha());
               for (int y = prevRect.y(); y < prevRect.bottom(); ++y) {
                   unsigned* const currentRow =
                       buffer->bytes().data() + (y * m_size.width());