Bug 16508 – Regression: Transparency mis-decoded in interlaced GIFs

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Peter Kasting]

GIFImageDecoder.cpp (not used by Safari, but used by various other WebKit ports) has a bug I introduced several weeks ago which causes it to mis-decode transparent sections of interlaced GIFs, by not overwriting earlier rows' opaque pixels with later rows' transparent ones.

It's not too tricky to fix this.  Patch coming shortly.

[Peter Kasting]

Created attachment 17978 [details]
patch v1

Sometimes we need to overwrite our buffer with transparent pixels, and sometimes we need to not do it.  This lets the GIFImageReader code tell us which one is the case.

[Mark Rowe (bdash)]

*** Bug 16504 has been marked as a duplicate of this bug. ***

[Alp Toker]

Comment on attachment 17978 [details]
patch v1

I did notice this regression. Patch looks sane, but haven't had the opportunity to review it closely yet and it does break the GTK+ build at least:

g++ -c -pipe -D_REENTRANT -I/usr/include -Wall -W -Wcast-align -Wchar-subscripts -Wformat-security -Wmissing-format-attribute -Wpointer-arith -Wwrite-strings -Wno-format-y2k -Wno-unused-parameter -Wundef -fno-exceptions -fno-rtti -Wreturn-type -fno-strict-aliasing -O2 -I/usr/include/cairo -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/cairo -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2 -I/usr/include/libpng12 -pthread -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/libxml2 -I/usr/include/libxml2 -fvisibility=hidden -fvisibility-inlines-hidden -fPIC -DQT_SHARED -DBUILDING_CAIRO__=1 -DBUILDING_GTK__=1 -DUSE_SYSTEM_MALLOC -DNDEBUG -DHAVE_STDINT_H -DBUILD_WEBKIT -DENABLE_DATABASE=1 -DENABLE_ICONDATABASE=1 -DENABLE_XPATH=1 -DENABLE_XSLT=1 -DENABLE_SVG=0 -DENABLE_VIDEO=0 -DWTF_CHANGES=1 -DBUILDING_GTK__ -I/usr/share/qt4/mkspecs/linux-g++ -I../../../WebCore -I../../../../WebKit -I../../../WebCore/platform/gtk -I../../../WebCore/platform/network/curl -I../../../WebCore/platform/graphics/cairo -I/home/alp/Projects/webkit/ngit/WebKit/WebCore/loader/gtk -I../../../WebCore/page/gtk -I../../../WebKit/gtk/WebView -I../../../WebKit/gtk/WebCoreSupport -I../../../JavaScriptCore -I../../../JavaScriptCore/kjs -I../../../JavaScriptCore/bindings -I../../../JavaScriptCore/bindings/c -I../../../JavaScriptCore/wtf -I../../../JavaScriptCore/ForwardingHeaders -I../../../WebCore -I../../../WebCore/ForwardingHeaders -I../../../WebCore/platform -I../../../WebCore/platform/network -I../../../WebCore/platform/graphics -I../../../WebCore/loader -I../../../WebCore/page -I../../../WebCore/css -I../../../WebCore/dom -I../../../WebCore/bridge -I../../../WebCore/editing -I../../../WebCore/rendering -I../../../WebCore/history -I../../../WebCore/xml -I../../../WebCore/html -Itmp -Itmp -I../../../JavaScriptCore -I../../../JavaScriptCore/kjs -I../../../JavaScriptCore/bindings -I../../../JavaScriptCore/bindings/c -I../../../JavaScriptCore/wtf -I../../../JavaScriptCore/pcre -I../JavaScriptCore/kjs/tmp -I../../../WebCore/platform/gtk -I../../../WebCore/platform/graphics/gtk -I../../../WebCore/platform/graphics/cairo -I../../../WebCore/svg/graphics/cairo -I../../../WebCore/platform/network/curl -I../../../WebCore/platform/image-decoders -I../../../WebCore/platform/image-decoders/bmp -I../../../WebCore/platform/image-decoders/gif -I../../../WebCore/platform/image-decoders/ico -I../../../WebCore/platform/image-decoders/jpeg -I../../../WebCore/platform/image-decoders/png -I../../../WebCore/platform/image-decoders/xbm -I/home/alp/Projects/webkit/ngit/WebKit/WebCore/loader/gtk -I../../../WebCore/page/gtk -I../../../WebKit/gtk/WebCoreSupport -I../../../WebKit/gtk/WebView -I../../../WebCore -I../../../WebCore/ForwardingHeaders -I../../../../WebKit -I../../../JavaScriptCore/kjs -I../../../JavaScriptCore/bindings -I../../../JavaScriptCore/wtf -I../../../WebCore/platform -I../../../WebCore/platform/network -I../../../WebCore/platform/graphics -I../../../WebCore/svg/graphics -I../../../WebCore/svg/graphics/filters -I../../../WebCore/platform/sql -I../../../WebCore/platform/text -I../../../WebCore/storage -I../../../WebCore/loader -I../../../WebCore/loader/icon -I../../../WebCore/css -I../../../WebCore/dom -I../../../WebCore/page -I../../../WebCore/bridge -I../../../WebCore/editing -I../../../WebCore/rendering -I../../../WebCore/history -I../../../WebCore/xml -I../../../WebCore/html -I../../../WebCore/bindings/js -I../../../WebCore/svg -I../../../WebCore/platform/image-decoders -I/usr/X11R6/include -I../../../WebCore -I. -o tmp/GIFImageDecoder.o ../../../WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp
../../../WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp: In member function ‘virtual WebCore::RGBA32Buffer* WebCore::GIFImageDecoder::frameBufferAtIndex(size_t)’:
../../../WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:161: warning: comparison between signed and unsigned integer expressions
../../../WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp: In member function ‘void WebCore::GIFImageDecoder::haveDecodedRow(unsigned int, unsigned char*, unsigned char*, unsigned int, unsigned int, bool)’:
../../../WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:329: error: no matching function for call to ‘WebCore::RGBA32Buffer::setRGBA(unsigned int*&, int, int, int, int)’
../../../WebCore/platform/image-decoders/ImageDecoder.h:73: note: candidates are: static void WebCore::RGBA32Buffer::setRGBA(unsigned int&, unsigned int, unsigned int, unsigned int, unsigned int)
make: *** [tmp/GIFImageDecoder.o] Error 1

[Alp Toker]

*** Bug 16340 has been marked as a duplicate of this bug. ***

[Peter Kasting]

Created attachment 18015 [details]
patch v2

Fix stupid typo in patch v1.  I left off a *.

[Alp Toker]

Comment on attachment 18015 [details]
patch v2

r=me

Thanks Peter!

[Alp Toker]

Landed in r28925.