Diff
Modified: trunk/Source/WebCore/ChangeLog (144960 => 144961)
--- trunk/Source/WebCore/ChangeLog 2013-03-06 20:05:33 UTC (rev 144960)
+++ trunk/Source/WebCore/ChangeLog 2013-03-06 20:09:06 UTC (rev 144961)
@@ -1,3 +1,36 @@
+2013-03-06 Alpha Lam <[email protected]>
+
+ More cleanup in GIFImageReader
+ https://bugs.webkit.org/show_bug.cgi?id=111137
+
+ Reviewed by Stephen White.
+
+ Refactor GIFImageReaderReader with the following changes:
+ + Separate GIFLZWContext for decoding states.
+ + Replace unsigned char* with Vector<unsigned char>
+
+ There is no change in code behavior and just refactoring.
+
+ No new tests. This is covered by existing GIFImageReaderTest.
+ I also did a local testing on a 50k image corpus and showed no regression.
+
+ * platform/image-decoders/gif/GIFImageDecoder.cpp:
+ (WebCore::GIFImageDecoder::haveDecodedRow):
+ * platform/image-decoders/gif/GIFImageDecoder.h:
+ (GIFImageDecoder):
+ * platform/image-decoders/gif/GIFImageReader.cpp:
+ (GIFImageReader::outputRow):
+ (GIFImageReader::doLZW):
+ (GIFImageReader::decodeInternal):
+ (GIFImageReader::prepareLZWContext):
+ * platform/image-decoders/gif/GIFImageReader.h:
+ (GIFFrameContext):
+ (GIFFrameContext::GIFFrameContext):
+ (GIFFrameContext::~GIFFrameContext):
+ (GIFLZWContext):
+ (GIFLZWContext::GIFLZWContext):
+ (GIFImageReader):
+
2013-03-06 Tim Horton <[email protected]>
Fix typo'd MainThreadScrollingBecauseOfStyleIndictaion
Modified: trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp (144960 => 144961)
--- trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp 2013-03-06 20:05:33 UTC (rev 144960)
+++ trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp 2013-03-06 20:09:06 UTC (rev 144961)
@@ -193,20 +193,20 @@
}
}
-bool GIFImageDecoder::haveDecodedRow(unsigned frameIndex, unsigned char* rowBuffer, unsigned char* rowEnd, unsigned rowNumber, unsigned repeatCount, bool writeTransparentPixels)
+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();
// 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
- // that (rowEnd - rowBuffer) == (size().width() - frameContext->xOffset), so
+ // that width == (size().width() - frameContext->xOffset), so
// we must ensure we don't run off the end of either the source data or the
// row's X-coordinates.
int xBegin = upperBoundScaledX(frameContext->xOffset);
int yBegin = upperBoundScaledY(frameContext->yOffset + rowNumber);
- int xEnd = lowerBoundScaledX(std::min(static_cast<int>(frameContext->xOffset + (rowEnd - rowBuffer)), size().width()) - 1, xBegin + 1) + 1;
+ int xEnd = lowerBoundScaledX(std::min(static_cast<int>(frameContext->xOffset + width), size().width()) - 1, xBegin + 1) + 1;
int yEnd = lowerBoundScaledY(std::min(static_cast<int>(frameContext->yOffset + rowNumber + repeatCount), size().height()) - 1, yBegin + 1) + 1;
- if (!rowBuffer || (xBegin < 0) || (yBegin < 0) || (xEnd <= xBegin) || (yEnd <= yBegin))
+ if (rowBuffer.isEmpty() || (xBegin < 0) || (yBegin < 0) || (xEnd <= xBegin) || (yEnd <= yBegin))
return true;
// Get the colormap.
@@ -230,7 +230,7 @@
ImageFrame::PixelData* currentAddress = buffer.getAddr(xBegin, yBegin);
// Write one row's worth of data into the frame.
for (int x = xBegin; x < xEnd; ++x) {
- const unsigned char sourceValue = *(rowBuffer + (m_scaled ? m_scaledColumns[x] : x) - frameContext->xOffset);
+ const unsigned char sourceValue = rowBuffer[(m_scaled ? m_scaledColumns[x] : x) - frameContext->xOffset];
if ((!frameContext->isTransparent || (sourceValue != frameContext->tpixel)) && (sourceValue < colorMapSize)) {
const size_t colorIndex = static_cast<size_t>(sourceValue) * 3;
buffer.setRGBA(currentAddress, colorMap[colorIndex], colorMap[colorIndex + 1], colorMap[colorIndex + 2], 255);
Modified: trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h (144960 => 144961)
--- trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h 2013-03-06 20:05:33 UTC (rev 144960)
+++ trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h 2013-03-06 20:09:06 UTC (rev 144961)
@@ -56,7 +56,7 @@
virtual void clearFrameBufferCache(size_t clearBeforeFrame);
// Callbacks from the GIF reader.
- bool haveDecodedRow(unsigned frameIndex, unsigned char* rowBuffer, unsigned char* rowEnd, unsigned rowNumber, unsigned repeatCount, bool writeTransparentPixels);
+ bool haveDecodedRow(unsigned frameIndex, const Vector<unsigned char>& rowBuffer, size_t width, size_t rowNumber, unsigned repeatCount, bool writeTransparentPixels);
bool frameComplete(unsigned frameIndex, unsigned frameDuration, ImageFrame::FrameDisposalMethod disposalMethod);
void gifComplete();
Modified: trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp (144960 => 144961)
--- trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp 2013-03-06 20:05:33 UTC (rev 144960)
+++ trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp 2013-03-06 20:09:06 UTC (rev 144961)
@@ -149,12 +149,10 @@
// CALLBACK: Let the client know we have decoded a row.
if (m_client && m_frameContext
- && !m_client->haveDecodedRow(m_imagesCount - 1, m_frameContext->rowbuf, m_frameContext->rowend,
+ && !m_client->haveDecodedRow(m_imagesCount - 1, m_lzwContext.rowBuffer, m_frameContext->width,
drowStart, drowEnd - drowStart + 1, m_frameContext->progressiveDisplay && m_frameContext->interlaced && m_frameContext->ipass > 1))
return false;
- m_frameContext->rowp = m_frameContext->rowbuf;
-
if (!m_frameContext->interlaced)
m_frameContext->irow++;
else {
@@ -213,30 +211,25 @@
// Copy all the decoder state variables into locals so the compiler
// won't worry about them being aliased. The locals will be homed
// back into the GIF decoder structure when we exit.
- int avail = m_frameContext->avail;
- int bits = m_frameContext->bits;
+ int avail = m_lzwContext.avail;
+ int bits = m_lzwContext.bits;
size_t cnt = bytesInBlock;
- int codesize = m_frameContext->codesize;
- int codemask = m_frameContext->codemask;
- int oldcode = m_frameContext->oldcode;
- int clearCode = m_frameContext->clearCode;
- unsigned char firstchar = m_frameContext->firstchar;
- int datum = m_frameContext->datum;
+ int codesize = m_lzwContext.codesize;
+ int codemask = m_lzwContext.codemask;
+ int oldcode = m_lzwContext.oldcode;
+ int clearCode = m_lzwContext.clearCode;
+ unsigned char firstchar = m_lzwContext.firstchar;
+ int datum = m_lzwContext.datum;
- if (!m_frameContext->prefix) {
- m_frameContext->prefix = new unsigned short[MAX_BITS];
- memset(m_frameContext->prefix, 0, MAX_BITS * sizeof(short));
- }
+ Vector<unsigned short>& prefix = m_lzwContext.prefix;
+ Vector<unsigned char>& suffix = m_lzwContext.suffix;
+ Vector<unsigned char>& stack = m_lzwContext.stack;
+ size_t stackp = m_lzwContext.stackp;
+ size_t rowp = m_lzwContext.rowPosition;
+ size_t width = m_frameContext->width;
+ size_t rowsRemaining = m_frameContext->rowsRemaining;
- unsigned short *prefix = m_frameContext->prefix;
- unsigned char *stackp = m_frameContext->stackp;
- unsigned char *suffix = m_frameContext->suffix;
- unsigned char *stack = m_frameContext->stack;
- unsigned char *rowp = m_frameContext->rowp;
- unsigned char *rowend = m_frameContext->rowend;
- unsigned rowsRemaining = m_frameContext->rowsRemaining;
-
- if (rowp == rowend)
+ if (rowp == width)
return true;
#define OUTPUT_ROW \
@@ -244,7 +237,8 @@
if (!outputRow()) \
return false; \
rowsRemaining--; \
- rowp = m_frameContext->rowp; \
+ rowp = 0; \
+ m_lzwContext.rowPosition = 0; \
if (!rowsRemaining) \
goto END; \
} while (0)
@@ -279,8 +273,8 @@
}
if (oldcode == -1) {
- *rowp++ = suffix[code];
- if (rowp == rowend)
+ m_lzwContext.rowBuffer[rowp++] = suffix[code];
+ if (rowp == width)
OUTPUT_ROW;
firstchar = oldcode = code;
@@ -289,29 +283,29 @@
incode = code;
if (code >= avail) {
- *stackp++ = firstchar;
+ stack[stackp++] = firstchar;
code = oldcode;
- if (stackp == stack + MAX_BITS)
+ if (stackp == MAX_BYTES)
return m_client ? m_client->setFailed() : false;
}
while (code >= clearCode) {
- if (code >= MAX_BITS || code == prefix[code])
+ if (code >= MAX_BYTES || code == prefix[code])
return m_client ? m_client->setFailed() : false;
// Even though suffix[] only holds characters through suffix[avail - 1],
// allowing code >= avail here lets us be more tolerant of malformed
- // data. As long as code < MAX_BITS, the only risk is a garbled image,
+ // data. As long as code < MAX_BYTES, the only risk is a garbled image,
// which is no worse than refusing to display it.
- *stackp++ = suffix[code];
+ stack[stackp++] = suffix[code];
code = prefix[code];
- if (stackp == stack + MAX_BITS)
+ if (stackp == MAX_BYTES)
return m_client ? m_client->setFailed() : false;
}
- *stackp++ = firstchar = suffix[code];
+ stack[stackp++] = firstchar = suffix[code];
// Define a new codeword in the dictionary.
if (avail < 4096) {
@@ -331,24 +325,24 @@
// Copy the decoded data out to the scanline buffer.
do {
- *rowp++ = *--stackp;
- if (rowp == rowend)
+ m_lzwContext.rowBuffer[rowp++] = stack[--stackp];
+ if (rowp == width)
OUTPUT_ROW;
- } while (stackp > stack);
+ } while (stackp > 0);
}
}
END:
// Home the local copies of the GIF decoder state variables.
- m_frameContext->avail = avail;
- m_frameContext->bits = bits;
- m_frameContext->codesize = codesize;
- m_frameContext->codemask = codemask;
- m_frameContext->oldcode = oldcode;
- m_frameContext->firstchar = firstchar;
- m_frameContext->datum = datum;
- m_frameContext->stackp = stackp;
- m_frameContext->rowp = rowp;
+ m_lzwContext.avail = avail;
+ m_lzwContext.bits = bits;
+ m_lzwContext.codesize = codesize;
+ m_lzwContext.codemask = codemask;
+ m_lzwContext.oldcode = oldcode;
+ m_lzwContext.firstchar = firstchar;
+ m_lzwContext.datum = datum;
+ m_lzwContext.stackp = stackp;
+ m_lzwContext.rowPosition = rowp;
m_frameContext->rowsRemaining = rowsRemaining;
return true;
}
@@ -390,41 +384,13 @@
break;
case GIFLZWStart: {
- // Initialize LZW parser/decoder.
- int datasize = *currentComponent;
-
- // Since we use a codesize of 1 more than the datasize, we need to ensure
- // that our datasize is strictly less than the MAX_LZW_BITS value (12).
- // This sets the largest possible codemask correctly at 4095.
- if (datasize >= MAX_LZW_BITS)
- return m_client ? m_client->setFailed() : false;
- int clearCode = 1 << datasize;
- if (clearCode >= MAX_BITS)
- return m_client ? m_client->setFailed() : false;
-
- if (m_frameContext) {
+ if (query == GIFImageDecoder::GIFFullQuery) {
+ int datasize = *currentComponent;
m_frameContext->datasize = datasize;
- m_frameContext->clearCode = clearCode;
- m_frameContext->avail = m_frameContext->clearCode + 2;
- m_frameContext->oldcode = -1;
- m_frameContext->codesize = m_frameContext->datasize + 1;
- m_frameContext->codemask = (1 << m_frameContext->codesize) - 1;
- m_frameContext->datum = m_frameContext->bits = 0;
-
- // Init the tables.
- if (!m_frameContext->suffix)
- m_frameContext->suffix = new unsigned char[MAX_BITS];
-
- // Clearing the whole suffix table lets us be more tolerant of bad data.
- memset(m_frameContext->suffix, 0, MAX_BITS);
- for (int i = 0; i < m_frameContext->clearCode; i++)
- m_frameContext->suffix[i] = i;
-
- if (!m_frameContext->stack)
- m_frameContext->stack = new unsigned char[MAX_BITS];
- m_frameContext->stackp = m_frameContext->stack;
+ // Initialize LZW parser/decoder.
+ if (!m_lzwContext.prepareToDecode(m_screenWidth, datasize))
+ return m_client ? m_client->setFailed() : false;
}
-
GETN(1, GIFSubBlock);
break;
}
@@ -712,16 +678,8 @@
/* This case will never be taken if this is the first image */
/* being decoded. If any of the later images are larger */
/* than the screen size, we need to reallocate buffers. */
- if (m_screenWidth < width) {
- /* XXX Deviant! */
- delete []m_frameContext->rowbuf;
- m_screenWidth = width;
- m_frameContext->rowbuf = new unsigned char[m_screenWidth];
- } else if (!m_frameContext->rowbuf)
- m_frameContext->rowbuf = new unsigned char[m_screenWidth];
+ m_screenWidth = std::max(m_screenWidth, width);
- if (!m_frameContext->rowbuf)
- return m_client ? m_client->setFailed() : false;
if (m_screenHeight < height)
m_screenHeight = height;
@@ -748,8 +706,6 @@
// Clear state from last image.
m_frameContext->irow = 0;
m_frameContext->rowsRemaining = m_frameContext->height;
- m_frameContext->rowend = m_frameContext->rowbuf + m_frameContext->width;
- m_frameContext->rowp = m_frameContext->rowbuf;
// bits per pixel is currentComponent[8]&0x07
}
@@ -842,3 +798,37 @@
ASSERT(remainingBytes <= m_data->size());
m_bytesRead = m_data->size() - remainingBytes;
}
+
+bool GIFLZWContext::prepareToDecode(unsigned rowWidth, int datasize)
+{
+ // Since we use a codesize of 1 more than the datasize, we need to ensure
+ // that our datasize is strictly less than the MAX_LZW_BITS value (12).
+ // This sets the largest possible codemask correctly at 4095.
+ if (datasize >= MAX_LZW_BITS)
+ return false;
+ clearCode = 1 << datasize;
+ if (clearCode >= MAX_BYTES)
+ return false;
+
+ avail = clearCode + 2;
+ oldcode = -1;
+ codesize = datasize + 1;
+ codemask = (1 << codesize) - 1;
+ datum = bits = 0;
+
+ // Initialize the tables lazily, this allows frame count query to use less memory.
+ suffix.resize(MAX_BYTES);
+ stack.resize(MAX_BYTES);
+ prefix.resize(MAX_BYTES);
+
+ // Initialize output row buffer.
+ rowBuffer.resize(rowWidth);
+ rowPosition = 0;
+
+ // Clearing the whole suffix table lets us be more tolerant of bad data.
+ suffix.fill(0);
+ for (int i = 0; i < clearCode; i++)
+ suffix[i] = i;
+ stackp = 0;
+ return true;
+}
Modified: trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.h (144960 => 144961)
--- trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.h 2013-03-06 20:05:33 UTC (rev 144960)
+++ trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.h 2013-03-06 20:09:06 UTC (rev 144961)
@@ -42,9 +42,10 @@
// so we will too.
#include "GIFImageDecoder.h"
#include "SharedBuffer.h"
+#include <wtf/Vector.h>
#define MAX_LZW_BITS 12
-#define MAX_BITS 4097 /* 2^MAX_LZW_BITS+1 */
+#define MAX_BYTES 4097 /* 2^MAX_LZW_BITS+1 */
#define MAX_COLORS 256
#define GIF_COLORS 3
@@ -74,28 +75,14 @@
GIFConsumeComment
};
+// Frame output state machine.
struct GIFFrameContext {
WTF_MAKE_FAST_ALLOCATED;
public:
- // LZW decoder state machine.
- unsigned char *stackp; // Current stack pointer.
int datasize;
- int codesize;
- int codemask;
- int clearCode; // Codeword used to trigger dictionary reset.
- int avail; // Index of next available slot in dictionary.
- int oldcode;
- unsigned char firstchar;
- int bits; // Number of unread bits in "datum".
- int datum; // 32-bit input buffer.
-
- // Output state machine.
int ipass; // Interlace pass; Ranges 1-4 if interlaced.
- unsigned rowsRemaining; // Rows remaining to be output.
- unsigned irow; // Current output row, starting at zero.
- unsigned char *rowbuf; // Single scanline temporary buffer.
- unsigned char *rowend; // Pointer to end of rowbuf.
- unsigned char *rowp; // Current output pointer.
+ size_t rowsRemaining; // Rows remaining to be output.
+ size_t irow; // Current output row, starting at zero.
// Parameters for image frame currently being decoded.
unsigned xOffset;
@@ -114,27 +101,11 @@
unsigned delayTime; // Display time, in milliseconds, for this image in a multi-image GIF.
- unsigned short* prefix; // LZW decoding tables.
- unsigned char* suffix; // LZW decoding tables.
- unsigned char* stack; // Base of LZW decoder stack.
-
GIFFrameContext()
- : stackp(0)
- , datasize(0)
- , codesize(0)
- , codemask(0)
- , clearCode(0)
- , avail(0)
- , oldcode(0)
- , firstchar(0)
- , bits(0)
- , datum(0)
+ : datasize(0)
, ipass(0)
, rowsRemaining(0)
, irow(0)
- , rowbuf(0)
- , rowend(0)
- , rowp(0)
, xOffset(0)
, yOffset(0)
, width(0)
@@ -148,21 +119,51 @@
, interlaced(false)
, isTransparent(false)
, delayTime(0)
- , prefix(0)
- , suffix(0)
- , stack(0)
{
}
~GIFFrameContext()
{
- delete [] rowbuf;
- delete [] prefix;
- delete [] suffix;
- delete [] stack;
}
};
+// LZW decoder state machine.
+// FIXME: Make this a class and hide private members.
+struct GIFLZWContext {
+ WTF_MAKE_FAST_ALLOCATED;
+public:
+ GIFLZWContext()
+ : stackp(0)
+ , codesize(0)
+ , codemask(0)
+ , clearCode(0)
+ , avail(0)
+ , oldcode(0)
+ , firstchar(0)
+ , bits(0)
+ , datum(0)
+ , rowPosition(0)
+ { }
+
+ bool prepareToDecode(unsigned rowWidth, int datasize);
+
+ size_t stackp; // Current stack pointer.
+ int codesize;
+ int codemask;
+ int clearCode; // Codeword used to trigger dictionary reset.
+ int avail; // Index of next available slot in dictionary.
+ int oldcode;
+ unsigned char firstchar;
+ int bits; // Number of unread bits in "datum".
+ int datum; // 32-bit input buffer.
+ size_t rowPosition;
+
+ Vector<unsigned short> prefix;
+ Vector<unsigned char> suffix;
+ Vector<unsigned char> stack;
+ Vector<unsigned char> rowBuffer; // Single scanline temporary buffer.
+};
+
class GIFImageReader {
WTF_MAKE_FAST_ALLOCATED;
public:
@@ -249,6 +250,8 @@
GIFFrameContext* m_frameContext;
+ GIFLZWContext m_lzwContext;
+
RefPtr<WebCore::SharedBuffer> m_data;
};
