As discussed in bug 331298, a macro in nsColor to help in construction of valid premultiplied ARGB will help to prevent issues in the different image decoders (see bug 360000 and others). #define NS_CAIRO_PIXEL(a,r,g,b) \ ((a) == 0x00) ? 0x00000000 : \ ((a) == 0xFF) ? ((0xFF << 24) | ((r) << 16) | ((g) << 8) | (b)) \ : ((a) << 24) | \ (NS_PREMULTIPLY(r,a) << 16) | \ (NS_PREMULTIPLY(g,a) << 8) | \ (NS_PREMULTIPLY(b,a)) This macro performs the trick, making sure that for zero alpha and opaque items the complex calculations are not performed. Furthermore by doing this as a macro, this makes the compiler when the a is constant to compile the specific path. So NS_CAIRO_PIXEL(0,r,g,b) compiles to 0, and NS_CAIRO_PIXEL(0xFF,r,g,b) compiles to 0xFF000000 | r<<16 | g<<6 | b This all prevents the need for copying the complex code into the different image decoders. See the attached patch where the ugly premultiplication stuff is replaced by NS_CAIRO_PIXEL in the PNG decoder. This even results in some improvement in the number of instructions per row loop: OLD NEW PNGDecoder: Case 1: 18 13 Case 2: 25 17 Old: one stack variable reference Case 3: 58 45 Old: four stack variable references. New: One stack variable ref

Comment on attachment 251215 [details] [diff] [review] V1: Patch to implement NS_CAIRO_PIXEL and to apply it to the PNG decoder this should probably go in gfx/thebes/public/gfxColor.h as we hope to get rid of everything in gfx/public and gfx/src some day...

Moved the macro's to gfxColor.h Also added better comments for each of the macros, whilst also used 'GFX_' as prefix.

Comment on attachment 251368 [details] [diff] [review] V2: add the macro's to gfxColor.h Vlad, could you also check if this is ok?

Comment on attachment 251368 [details] [diff] [review] V2: add the macro's to gfxColor.h If stuart's ok with the imagelib changes, it looks fine to me -- note that I had problems with the fast divide by 255 macro a while back, can't remember with which compiler. Just putting that note here in case we start seeing any weirdness. (Colors were looking corrupted.)

Who can do the checkin for me?

gfx/thebes/public/gfxColor.h 1.12 modules/libpr0n/decoders/png/Makefile.in 1.19 modules/libpr0n/decoders/png/nsPNGDecoder.cpp 1.56

Is this something that can be unit tested (via reftest perhaps?)

Thanks for the checkin. And indeed attachment #3 [details] [diff] [review] is good material for automated testing, but it seems more something for Mochitest.