Skip to site navigation (Press enter)

[webkit-changes] [152352] trunk

commit-queue Wed, 03 Jul 2013 06:53:43 -0700

Title: [152352] trunk

Revision: 152352
Author: [email protected]
Date: 2013-07-03 06:54:45 -0700 (Wed, 03 Jul 2013)

Log Message

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 <[email protected]> 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.

Modified Paths

trunk/Source/WebCore/ChangeLog
trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp
trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h
trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp

Added Paths

trunk/ManualTests/animated-gif-dispose-background.html
trunk/ManualTests/resources/dispose-background.gif

Diff

Added: trunk/ManualTests/animated-gif-dispose-background.html (0 => 152352)


--- trunk/ManualTests/animated-gif-dispose-background.html	                        (rev 0)
+++ trunk/ManualTests/animated-gif-dispose-background.html	2013-07-03 13:54:45 UTC (rev 152352)
@@ -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="" />
+</body>
+</html>

Added: trunk/ManualTests/resources/dispose-background.gif (0 => 152352)


--- trunk/ManualTests/resources/dispose-background.gif	                        (rev 0)
+++ trunk/ManualTests/resources/dispose-background.gif	2013-07-03 13:54:45 UTC (rev 152352)
@@ -0,0 +1,9 @@
+GIF89add\xF0\x90\xFF\x87\xCE\xEB!\xF9d!\xFFNETSCAPE2.0,dd\xFE\x8C\x8F\xA9\xCB\xED\xA3\x9C\xB4ڋ\xB3޼\xFB\x86\xE2H\x96扦\xEA\xB4\xEE\xEB\xAE2\xD7\xF6;\x97\xF7η\xB9\xD7\xA1qH\x94\x97\xC8d\x83	\xE59Ѫtj\xB0joӭ\x97\xFB\xFB\x8Ak\xB9\xB1\xB7:\xAB}\xA9\xB5厛\xE2t\xFD>\xBA\xD7Cz\xFC\xA7\xAF\xF7\xB8\xC71\xB8a؇\x98\xE8W\xC4xx\xF1Y!\xA9hQ99\x81٨\xB9\xC9\xE1\xF9	JJZ\xFAp\x8A\xEA\xA0*\xA7Ժ\xD6	{\xF6:\xABfjK;\x9A\xAB\x9B\xCAk\x86\xFB\xFB,\xBCE\lu\x8C\xA5\xBC\xCC\xD4\xEC|-4M\xDDc}\xBDS\xABm$\xDB]\xCD
+\x8E->\xBE]nnC\x91~\xF5\xCDC\xF9\xAE/sYoO\x8F\xEFX\x9F\x81\xA0\xA1_@y\x85\xD81'h\x85|\xB4\xE5q\xF80\xDAgp\x90\xA9(&\xE3W.[Dfu!\x85%\x8B\xA7\x90*\x91T\xF0\xE8$\x83A*}\xB9j\x99\xED\xCCHZf&lg3\xA7Ν<{\xFA\xFC	4\xE8\x89!\xF9d,
+  \x83\xFF\xFF\x80\xB2""\xA0R-\xFF\xFF\xFF.\x8BW\xFF\xFF\xFF\xFF\xFF\xFF\xEEP\xC9I\xAB\xBD8\x83\xCDA\xFE\xD3v# \x90h\x91\xA5p\x8C\x82{\x96\xAA\x84\x921\x9A\xFB	\x96\xA3\x83a \xDC\xC9\xE4\xC1\x88FGc\xF2\x98,"q\xCD
+\x8Ah%^\xA9ӫ5;\x81J\x87\x80\xEB! F\x86\xC9\xA34\xB0	\xD0\xD7v:\xFBm\x91\x93\xECwyz\x80Gp
+FA\x80x#\x83y\x87O,#		\x97	\x81\x9D\x93\x97\x99\x97\x9A\x9C\xA5\x9Ck\x876A\x99\xA4\xAF\x9C\xAE\xB1\xA6\xAD\xADD\xBA\x9B\xAE \x99F\xB8\xB9\xB8\xBC\xA5\xAA+~\xBAȻ\xBA56F\x9B\xA4\xD2\xD36\xC2\xDF؛\xCB\xC1\xDF\xE5\xE1\xE3*\xDE\xE6\xA4\xE8\xE9\xE5\xB2\xC0\xDCN\xEB\xED\xD3)\xF2N\xF7\xF5\xF6!\xF9Y\xF8\xD5\xD8'017\xAC\xF2O\xBE!\xF9d,#  \x82\xB2""\xBE\xBE\xBE\xFF\xFF\xFF\xFFp\x80\x90\xDB8\xBA\xDC\xFE0\x8E@\x90؁"DYp\xE1]\xA1P
+e)\x80je9\xEBV\xEC\xF7\x9B\xB4\xCA\xEEs\xABm~\xB3	\xC9\xF4l`\xB1υR(}\xA2\x97\xA74\xFA\xAC\xA1B\xB8\x8E\xB3\xB6\xC9w\\x83Y\xC7I\xEA\xE4\xAEK\xE4q1g\xEB\xFB\xF5p~(T<Ncu<\x83T16zmeEGnz\x90\x96Mwxg\x95x!wt_Aw4\x94mE\xA5jPIP~6Pt"b?\x92}\x8B\xB0\xA0\xB9\xB6)
+\xB3\x95\x9A\xC1NP\x92\x98\xC6\xC1\x8DVDU\xCC\xCDO^\x8A\xC7|K\xC0\xD7\xC2U0\xD2ܥD\xA1\xDCBx\xE5\xE6ݺ\xEA\xEB_Wk\xA8\xEBW\xE4\xBC\xE6f\xC3	!\xF9d,>2  \x82\xFF\xFF\x90\xFF\xFF\xFF\xFFp\x80\x90 \xB2\xAA\xFF\xFE8\xBA\xDC>\xBC	\xE5\x8AV!2\xD5D r\xDD\xC7l[G\xBED\xBB\xDE\xD4Ҭ\x89u\x80!\xD0*ʀ\xC4p>\x90\xB2C)#!'1\xF2\x88\x94ZC\xDA3B\x9C\xC62X\xAF[]1\xA53$\xA0H\x8BdPV\x90
+\x81\x82\x82\x9FCF/\x83vy\x86B<ib2\x83\x8Evx\x912s\x8AWS\x8E\x84k\x91&!m\x94|\x97\x98\x90\x86u!y{\x8A\x84\xA1\x8F\x9A\x862\x9B\xA2\xAB\x99w\xA4\x93\x9By\xB24\x98u\xADx\xAF\xA4\xB9\x8F\xB3\xBD\xB5\x92\xA6\xB8\xBCcw\xA2\xBE\x9C\x9B\x91\xBA̬\xC6y\xB7\x86ª\xBB\xCD#\x9B\xC0\xD1\xD3\xCA\xD1\xD8x\xDAuą\xDF!\xD1\xE8\xCB\xE3a\xC7.\xC9\xE2\x8F`\xB5\xE0\xA4\xF6TM\xDEٔM\xE9Ǥ]0\x81K\x82`\xF6\x8E`\x90\xD6\xEE\xD5x\xB8bĵ\x89\xAB8T\x91!\xF9d,
+7  \x81\xBE\xBE\xBE\xFF\xFF\xA5\xFF\xFF\xB8\x8C\x8F8\xB1\xCD\xDF\xC6JZ'c\x96\xA9?\x83]^\xA6\xB6\xA5\x95\xA7\x9C\xE2Z\x86بnl\x85\xBB\xF3!\xEBu\xEAt\xC2`F\xA4\xF0t\x90\xB5\x8C1I\xD3׏Zd\x8B\xC4Ƒ\xF1킕\xD2^\xA6\xF9F\xAB\x91\xA5\xF9\x85V\xB6s7\x8C\xCFdK\xE5\x93\xD3Fgpg\xE7U\xB6\xF2&\xF3\x97&Pg\xD0("`809\xD6ҧ\x88r\xA8eGxڕ\x93\xE9S\xE3\x99\xD5(\x94\xAA%V\xD9\xB9\x96\x88\xC7\xE9\xE8y\xE9\x9A\xEA\xC6\xB1x\xA0\x9A0\xCC5yK\xF6\xBB\xC9\xD19(\xCA+6\xDB\xFCS;
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (152351 => 152352)


--- trunk/Source/WebCore/ChangeLog	2013-07-03 12:59:55 UTC (rev 152351)
+++ trunk/Source/WebCore/ChangeLog	2013-07-03 13:54:45 UTC (rev 152352)
@@ -1,3 +1,26 @@
+2013-07-03  Balazs Kelemen  <[email protected]>
+
+        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  <[email protected]>
 
         TextureUnit code optimization

Modified: trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp (152351 => 152352)


--- trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp	2013-07-03 12:59:55 UTC (rev 152351)
+++ trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp	2013-07-03 13:54:45 UTC (rev 152352)
@@ -178,6 +178,23 @@
     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)

Modified: trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h (152351 => 152352)


--- trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h	2013-07-03 12:59:55 UTC (rev 152351)
+++ trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h	2013-07-03 13:54:45 UTC (rev 152352)
@@ -77,6 +77,7 @@
         // 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

Modified: trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp (152351 => 152352)


--- trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp	2013-07-03 12:59:55 UTC (rev 152351)
+++ trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp	2013-07-03 13:54:45 UTC (rev 152352)
@@ -370,7 +370,7 @@
     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 @@
                 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);
             }
         }
     }

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes