From 4f2a9994b6349b13873075b7b9945a63ee627ae9 Mon Sep 17 00:00:00 2001 From: "alp@webkit.org" Date: Tue, 27 Nov 2007 03:01:23 +0000 Subject: [PATCH] 2007-11-26 Peter Kasting 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 | 24 +++ .../platform/image-decoders/ImageDecoder.h | 16 +- .../image-decoders/gif/GIFImageDecoder.cpp | 193 ++++++++++-------- .../image-decoders/gif/GIFImageDecoder.h | 16 +- .../image-decoders/gif/GIFImageReader.cpp | 8 +- .../image-decoders/gif/GIFImageReader.h | 14 +- 6 files changed, 155 insertions(+), 116 deletions(-) diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index 3a0f95be3b33..54febc491936 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,3 +1,27 @@ +2007-11-26 Peter Kasting + + 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 Add a Color(CGColorRef) constructor diff --git a/WebCore/platform/image-decoders/ImageDecoder.h b/WebCore/platform/image-decoders/ImageDecoder.h index ed1d0542a05d..3df2e880a2a0 100644 --- a/WebCore/platform/image-decoders/ImageDecoder.h +++ b/WebCore/platform/image-decoders/ImageDecoder.h @@ -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. }; diff --git a/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp b/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp index b7ea6b2de261..c0b93b5895be 100644 --- a/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp +++ b/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp @@ -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(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() diff --git a/WebCore/platform/image-decoders/gif/GIFImageDecoder.h b/WebCore/platform/image-decoders/gif/GIFImageDecoder.h index 60d63e67b568..e2fc76717fa8 100644 --- a/WebCore/platform/image-decoders/gif/GIFImageDecoder.h +++ b/WebCore/platform/image-decoders/gif/GIFImageDecoder.h @@ -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; }; diff --git a/WebCore/platform/image-decoders/gif/GIFImageReader.cpp b/WebCore/platform/image-decoders/gif/GIFImageReader.cpp index b49915100936..0f9b2e9bb008 100644 --- a/WebCore/platform/image-decoders/gif/GIFImageReader.cpp +++ b/WebCore/platform/image-decoders/gif/GIFImageReader.cpp @@ -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) { diff --git a/WebCore/platform/image-decoders/gif/GIFImageReader.h b/WebCore/platform/image-decoders/gif/GIFImageReader.h index c0c543b03907..faa08d27459d 100644 --- a/WebCore/platform/image-decoders/gif/GIFImageReader.h +++ b/WebCore/platform/image-decoders/gif/GIFImageReader.h @@ -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; -- 2.36.0