Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20061108 Minefield/3.0a1 ID:2006110810 [cairo] repro: 1.open url result: no transparency, it should show only a red "O" not a white background The favicon is send by the server (no link in the html source) so I don't know what's so special about it that is fails. It works as expected in a non-cairo build

The URL of the favicon is: http://teletekst.nos.nl/favicon.ico When I save the icon to my windows desktop it is transparent except for the circle. But opening it in IE7 has the same effect as minefield. It shows the white when opening the icon in the content area and changing the default background color to something other than white. But two programs I looked at the icon with (XnView and Visual Studio 2002) don't show the icon as being transparent. But Eye of Gnome in Ubuntu does show the transparency but I don't see how to view the other versions of the icon.

yeah there is a problem with transparency on some images -- i'll fix it after some of the 1.9alpha bugs

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

I have a working patch to fix the transparancy decoding for ICO and BMP images in Cairo.

Detailed explanation to follow soon

Overview of the patch: Cairo: instead of doing SetImageData, use GetImageData to get the pointer to the image data and set the image data directly. Cairo: Get rid of image buffers and alpha buffers Cairo: Set transparency directly into the right place in the image Both: Get rid of row buffers Both: Optimize CalcBit functions Both: Use GetUInt32 inline function instead of the LITTLE_TO macro's Both: Only call OnDataAvailable per batch of rows instead of for every row. Both: Header decoding: only decode the values really used. Both: nsICODecoder: do alpha decoding in one step, instead of two steps. This saves rowBuffer allocation for both codepaths, and other buffers for Cairo. Also the alpha decoding for ICO and BMP is faster and really works.

Testcases: http://home.unet.nl/alfredkayser/ico.zip (use xxx.html to show all icons) http://home.unet.nl/alfredkayser/bmp.zip

Comment on attachment 249618 [details] [diff] [review] V2: Correctly last-minute typo and removed not-used return value from WriteRLE looks great. good job!

Can you separate out your changes into three bugs/patches (more if you feel it make for a more natural split): 1) whatever the fix for this bug was 2) removing extra buffers 3) miscellaneous code improvements

Tor, This patch only addresses the transparency thing for favicon. The issue only happened for 32bit icons so with real alpha transparency. The solution is thus premultiplication!

Tor, This patch only addresses the transparency thing for favicon. The issue only happened for 32bit icons so with real alpha transparency. The solution is thus premultiplication! This patch also makes sure the mAlphaBuffer is not allocaed for Cairo (as it is not used anyway), and that SetPixel really gets inlined in the right way (compensating for the PREMULTIPLY overhead)

Comment on attachment 251104 [details] [diff] [review] Patch to just fix the transparancy issue Thanks for splitting out the pertinent chunks.

mozilla/modules/libpr0n/decoders/bmp/nsBMPDecoder.h 1.23 mozilla/modules/libpr0n/decoders/bmp/nsICODecoder.cpp 1.40 mozilla/modules/libpr0n/decoders/bmp/nsICODecoder.h 1.14

Verified in build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070117 Minefield/3.0a2pre Verified with http://teletekst.nos.nl/ and with testcase from above (save as exmatrikulationsamt.de.ico)

Seems this may have cause regression to file folder icon's color in download manager (bug 367275 filed).