Bug 360000 - make 32-bit favicons with transparency work correctly in cairo

p=Alfred Kayser <alfredkayser@nl.ibm.com>, r=pavlov, sr=tor

modules/libpr0n/decoders/bmp/nsBMPDecoder.h

@@ -229,24 +229,22 @@ private:
 };
 
 /** Sets the pixel data in aDecoded to the given values.
- * The variable passed in as aDecoded will be moved on 3 bytes! */
+ * The variable passed in as aDecoded will be moved on 3 or 4 bytes! */
 inline void SetPixel(PRUint8*& aDecoded, PRUint8 aRed, PRUint8 aGreen, PRUint8 aBlue, PRUint8 aAlpha = 0xFF)
 {
 #if defined(MOZ_CAIRO_GFX)
-#ifdef IS_LITTLE_ENDIAN
+    // This is not skipping for pixels where aAlpha happen to be 0xFF
-  // BGRX
+    // This will also make the compiler skip this part completely when 
-  *aDecoded++ = aBlue;
+    // inlining SetPixel when aAlpha is not specified
-  *aDecoded++ = aGreen;
+    if (aAlpha != 0xFF) {
-  *aDecoded++ = aRed;
+#define DO_PREMULTIPLY(c_) ((PRUint16(c_) * PRUint16(aAlpha) + 127) / 255)
-  *aDecoded++ = aAlpha;
+        aBlue = DO_PREMULTIPLY(aBlue);
-#else
+        aGreen = DO_PREMULTIPLY(aGreen);
-  // XRGB
+        aRed = DO_PREMULTIPLY(aRed);
-  *aDecoded++ = aAlpha;
+#undef DO_PREMULTIPLY
-  *aDecoded++ = aRed;
+    }
-  *aDecoded++ = aGreen;
+    *(PRUint32*)aDecoded = (aAlpha << 24) | (aRed << 16) | (aGreen << 8) | aBlue;
-  *aDecoded++ = aBlue;
+    aDecoded += 4;
-#endif
-
 #else // MOZ_CAIRO_GFX
 
 #if defined(XP_MAC) || defined(XP_MACOSX)
@@ -266,11 +264,7 @@ inline void SetPixel(PRUint8*& aDecoded, PRUint8 aRed, PRUint8 aGreen, PRUint8 a
 
 inline void SetPixel(PRUint8*& aDecoded, PRUint8 idx, colorTable* aColors)
 {
-    PRUint8 red, green, blue;
+    SetPixel(aDecoded, aColors[idx].red, aColors[idx].green, aColors[idx].blue);
-    red = aColors[idx].red;
-    green = aColors[idx].green;
-    blue = aColors[idx].blue;
-    SetPixel(aDecoded, red, green, blue);
 }
 
 /** Sets two (or one if aCount = 1) pixels

modules/libpr0n/decoders/bmp/nsICODecoder.cpp

@@ -96,6 +96,7 @@ nsresult nsICODecoder::SetImageData()
   return NS_OK;
 }
 
+#ifndef MOZ_CAIRO_GFX
 nsresult nsICODecoder::SetAlphaData()
 {
   // Alpha data was already set if bpp == 32
@@ -131,6 +132,7 @@ nsresult nsICODecoder::SetAlphaData()
   free(alphaRow);
   return NS_OK;
 }
+#endif
 
 PRUint32 nsICODecoder::CalcAlphaRowSize()
 {
@@ -150,7 +152,9 @@ nsICODecoder::nsICODecoder()
   mHaveAlphaData = 0;
   mDecodingAndMask = PR_FALSE;
   mDecodedBuffer = nsnull;
+#ifndef MOZ_CAIRO_GFX
   mAlphaBuffer = nsnull;
+#endif
 }
 
 nsICODecoder::~nsICODecoder()
@@ -174,9 +178,11 @@ NS_IMETHODIMP nsICODecoder::Init(imgILoad *aLoad)
 
 NS_IMETHODIMP nsICODecoder::Close()
 {
-  mObserver->OnStopContainer(nsnull, mImage);
+  if (mObserver) {
-  mObserver->OnStopDecode(nsnull, NS_OK, nsnull);
+    mObserver->OnStopContainer(nsnull, mImage);
-  mObserver = nsnull;
+    mObserver->OnStopDecode(nsnull, NS_OK, nsnull);
+    mObserver = nsnull;
+  }
 
   mImage = nsnull;
   mFrame = nsnull;
@@ -197,7 +203,9 @@ NS_IMETHODIMP nsICODecoder::Close()
 
   mDecodingAndMask = PR_FALSE;
   free(mDecodedBuffer);
+#ifndef MOZ_CAIRO_GFX
   free(mAlphaBuffer);
+#endif
 
   return NS_OK;
 }
@@ -410,6 +418,7 @@ nsresult nsICODecoder::ProcessData(const char* aBuffer, PRUint32 aCount) {
       if (!mDecodedBuffer)
         return NS_ERROR_OUT_OF_MEMORY;
     }
+#ifndef MOZ_CAIRO_GFX
     PRUint32 alphaRowSize;
     mFrame->GetAlphaBytesPerRow(&alphaRowSize);
     nsAutoArrayPtr<PRUint8> alphaRow; // Will only be used if bpp == 32
@@ -418,6 +427,7 @@ nsresult nsICODecoder::ProcessData(const char* aBuffer, PRUint32 aCount) {
       if (!alphaRow)
         return NS_ERROR_OUT_OF_MEMORY;
     }
+#endif
 
     // Ensure memory has been allocated before decoding. If we get this far 
     // without allocated memory, the file is most likely invalid.
@@ -442,10 +452,11 @@ nsresult nsICODecoder::ProcessData(const char* aBuffer, PRUint32 aCount) {
         }
         if (rowSize == mRowBytes) {
             mCurLine--;
-            PRUint8* decoded = mDecodedBuffer + (mCurLine * mDirEntry.mWidth * GFXBYTESPERPIXEL);
+            PRUint8* d = mDecodedBuffer + (mCurLine * mDirEntry.mWidth * GFXBYTESPERPIXEL);
             PRUint8* p = mRow;
-            PRUint8* d = decoded;
+#ifndef MOZ_CAIRO_GFX
             PRUint8* alphaPos = alphaRow; // only used if bpp == 32
+#endif
             PRUint32 lpos = mDirEntry.mWidth;
             switch (mBIH.bpp) {
               case 1:
@@ -484,18 +495,24 @@ nsresult nsICODecoder::ProcessData(const char* aBuffer, PRUint32 aCount) {
                   p+=2;
                 }
                 break;
-              case 32:
               case 24:
                 while (lpos > 0) {
-#ifdef MOZ_CAIRO_GFX
-                  SetPixel(d, p[2], p[1], p[0], (mBIH.bpp == 32) ? p[3] : 0xff);
-#else
                   SetPixel(d, p[2], p[1], p[0]);
-#endif
                   p += 3;
                   --lpos;
-                  if (mBIH.bpp == 32)
+                }
-                    mHaveAlphaData |= *alphaPos++ = *p++; // Alpha value
+                break;
+              case 32:
+                while (lpos > 0) {
+#ifdef MOZ_CAIRO_GFX
+                  SetPixel(d, p[2], p[1], p[0], p[3]);
+                  mHaveAlphaData |= p[3]; // Alpha value
+#else
+                  SetPixel(d, p[2], p[1], p[0]);
+                  mHaveAlphaData |= *alphaPos++ = p[3]; // Alpha value
+#endif
+                  p += 4;
+                  --lpos;
                 }
                 break;
               default:
@@ -508,9 +525,11 @@ nsresult nsICODecoder::ProcessData(const char* aBuffer, PRUint32 aCount) {
               
             mRowBytes = 0;
 
+#ifndef MOZ_CAIRO_GFX
             // If 32 bit image, gotta set the alpha data here
             if (mBIH.bpp == 32)
               mFrame->SetAlphaData(alphaRow, alphaRowSize, mCurLine * alphaRowSize);
+#endif
         }
     } while (!mDecodingAndMask && aCount > 0);
 
@@ -580,7 +599,7 @@ nsresult nsICODecoder::ProcessData(const char* aBuffer, PRUint32 aCount) {
                 // We complement the value, since our method of storing
                 // transparency is opposite what Win32 uses in its masks.
                 *decoded = (idx & bit) ? 0x00 : 0xff;
-                decoded += 4;
+                decoded += GFXBYTESPERPIXEL;
               }
             }
 #else

modules/libpr0n/decoders/bmp/nsICODecoder.h

@@ -97,7 +97,9 @@ private:
   void ProcessInfoHeader();
 
   nsresult SetImageData();
+#ifndef MOZ_CAIRO_GFX
   nsresult SetAlphaData();
+#endif
 
   PRUint32 CalcAlphaRowSize();
 
@@ -106,9 +108,6 @@ private:
   nsCOMPtr<imgIContainer> mImage;
   nsCOMPtr<gfxIImageFrame> mFrame;
   
-  PRUint8 mHaveAlphaData;
-  PRPackedBool mDecodingAndMask;
-
   PRUint32 mPos;
   PRUint16 mNumIcons;
   PRUint16 mCurrIcon;
@@ -130,8 +129,13 @@ private:
   nsresult mStatus;
 
   PRUint8* mDecodedBuffer;
+#ifndef MOZ_CAIRO_GFX
   PRUint8* mAlphaBuffer;
+#endif
+
+  PRUint8 mHaveAlphaData;
   PRPackedBool mIsCursor;
+  PRPackedBool mDecodingAndMask;
 };