Bug 16508

Summary: Regression: Transparency mis-decoded in interlaced GIFs
Product: WebKit Reporter: Peter Kasting <pkasting>
Component: ImagesAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: sandshrew
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
patch v1
alp: review-
patch v2 alp: review+

Peter Kasting
Reported 2007-12-18 16:07:13 PST
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.
Attachments
patch v1 (4.19 KB, patch)
2007-12-18 18:16 PST, Peter Kasting
alp: review-
patch v2 (4.19 KB, patch)
2007-12-20 13:54 PST, Peter Kasting
alp: review+
Peter Kasting
Comment 1 2007-12-18 18:16:53 PST
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)
Comment 2 2007-12-18 18:39:43 PST
*** Bug 16504 has been marked as a duplicate of this bug. ***
Alp Toker
Comment 3 2007-12-19 09:19:40 PST
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
Comment 4 2007-12-20 13:25:21 PST
*** Bug 16340 has been marked as a duplicate of this bug. ***
Peter Kasting
Comment 5 2007-12-20 13:54:29 PST
Created attachment 18015 [details] patch v2 Fix stupid typo in patch v1. I left off a *.
Alp Toker
Comment 6 2007-12-20 19:55:35 PST
Comment on attachment 18015 [details] patch v2 r=me Thanks Peter!
Alp Toker
Comment 7 2007-12-20 19:56:24 PST
Landed in r28925.
Note You need to log in before you can comment on or make changes to this bug.