Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Gif: zero filling should use memset instead of setRGBA for every pixel

https://bugs.webkit.org/show_bug.cgi?id=118350

Patch by Balazs Kelemen <b.kelemen@samsung.com> on 2013-07-03
Reviewed by Allan Sandfeld Jensen.

No new tests. Actually it is not covered by existing tests. Surprisingly we haven't got pixel
tests for animated images. Given that this patch is pretty trivial I don't think it's worth the
cost to start introducing such tests.
I added a manual test: animated-gif-dispose-background.html.

GIFImageDecoder::initializeFrameBuffer use a loop to fill a subrect with tranparent pixels.
This is extremely ineffecient. The use case for this code path is not frequent on the web
but it's still better to fix it.

* platform/image-decoders/ImageDecoder.cpp:
(WebCore::ImageFrame::zeroFillFrameRect):
* platform/image-decoders/ImageDecoder.h:
* platform/image-decoders/gif/GIFImageDecoder.cpp:
(WebCore::GIFImageDecoder::initFrameBuffer):
Fixed indentation in addition.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@152352 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information...
commit f85baf20676665a741a97ef61ea2db7696c4e47b 1 parent 6e30983
commit-queue@webkit.org authored
View
6 ManualTests/animated-gif-dispose-background.html
@@ -0,0 +1,6 @@
+<html>
+<body>
+ <p>Animated gif with background dispose method. Animation frames live behind their rects filled with transparent pixels.</p>
+ <img src="./resources/dispose-background.gif" />
+</body>
+</html>
View
BIN  ManualTests/resources/dispose-background.gif
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
23 Source/WebCore/ChangeLog
@@ -1,3 +1,26 @@
+2013-07-03 Balazs Kelemen <b.kelemen@samsung.com>
+
+ Gif: zero filling should use memset instead of setRGBA for every pixel
+ https://bugs.webkit.org/show_bug.cgi?id=118350
+
+ Reviewed by Allan Sandfeld Jensen.
+
+ No new tests. Actually it is not covered by existing tests. Surprisingly we haven't got pixel
+ tests for animated images. Given that this patch is pretty trivial I don't think it's worth the
+ cost to start introducing such tests.
+ I added a manual test: animated-gif-dispose-background.html.
+
+ GIFImageDecoder::initializeFrameBuffer use a loop to fill a subrect with tranparent pixels.
+ This is extremely ineffecient. The use case for this code path is not frequent on the web
+ but it's still better to fix it.
+
+ * platform/image-decoders/ImageDecoder.cpp:
+ (WebCore::ImageFrame::zeroFillFrameRect):
+ * platform/image-decoders/ImageDecoder.h:
+ * platform/image-decoders/gif/GIFImageDecoder.cpp:
+ (WebCore::GIFImageDecoder::initFrameBuffer):
+ Fixed indentation in addition.
+
2013-07-03 Przemyslaw Szymanski <p.szymanski3@samsung.com>
TextureUnit code optimization
View
17 Source/WebCore/platform/image-decoders/ImageDecoder.cpp
@@ -178,6 +178,23 @@ void ImageFrame::zeroFillPixelData()
m_hasAlpha = true;
}
+void ImageFrame::zeroFillFrameRect(const IntRect& rect)
+{
+ ASSERT(IntRect(IntPoint(), m_size).contains(rect));
+
+ if (rect.isEmpty())
+ return;
+
+ size_t rectWidthInBytes = rect.width() * sizeof(PixelData);
+ PixelData* start = m_bytes + (rect.y() * width()) + rect.x();
+ for (int i = 0; i < rect.height(); ++i) {
+ memset(start, 0, rectWidthInBytes);
+ start += width();
+ }
+
+ setHasAlpha(true);
+}
+
bool ImageFrame::copyBitmapData(const ImageFrame& other)
{
if (this == &other)
View
1  Source/WebCore/platform/image-decoders/ImageDecoder.h
@@ -77,6 +77,7 @@ namespace WebCore {
// These do not touch other metadata, only the raw pixel data.
void clearPixelData();
void zeroFillPixelData();
+ void zeroFillFrameRect(const IntRect&);
// Makes this frame have an independent copy of the provided image's
// pixel data, so that modifications in one frame are not reflected in
View
15 Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp
@@ -370,7 +370,7 @@ bool GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
int top = upperBoundScaledY(frameRect.y());
int bottom = lowerBoundScaledY(frameRect.maxY(), top);
buffer->setOriginalFrameRect(IntRect(left, top, right - left, bottom - top));
-
+
if (!frameIndex) {
// This is the first frame, so we're not relying on any previous data.
if (!buffer->setSize(scaledSize().width(), scaledSize().height()))
@@ -407,15 +407,10 @@ bool GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
if (!buffer->setSize(bufferSize.width(), bufferSize.height()))
return setFailed();
} else {
- // Copy the whole previous buffer, then clear just its frame.
- if (!buffer->copyBitmapData(*prevBuffer))
- return setFailed();
- for (int y = prevRect.y(); y < prevRect.maxY(); ++y) {
- for (int x = prevRect.x(); x < prevRect.maxX(); ++x)
- buffer->setRGBA(x, y, 0, 0, 0, 0);
- }
- if ((prevRect.width() > 0) && (prevRect.height() > 0))
- buffer->setHasAlpha(true);
+ // Copy the whole previous buffer, then clear just its frame.
+ if (!buffer->copyBitmapData(*prevBuffer))
+ return setFailed();
+ buffer->zeroFillFrameRect(prevRect);
}
}
}
Please sign in to comment.
Something went wrong with that request. Please try again.