Bug 775793 - Clarify BMP decoding for unexpected alpha pixels - r=bbondy, a=akeybl
authorJeff Gilbert <jgilbert@mozilla.com>
Tue, 24 Jul 2012 17:02:22 -0700
changeset 81920 6fdfc850e249427a092030b7ec81a47773e668cc
parent 81919 b86437f03200b92fc991fce4d0e2eb8178fb5b9c
child 81921 bb1d806b2efa69a73dda642bdbc387fbe21653be
push id218
push userjgilbert@mozilla.com
push dateWed, 25 Jul 2012 00:03:31 +0000
reviewersbbondy, akeybl
bugs775793
milestone10.0.7esrpre
Bug 775793 - Clarify BMP decoding for unexpected alpha pixels - r=bbondy, a=akeybl
image/decoders/nsBMPDecoder.cpp
image/decoders/nsBMPDecoder.h
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -101,21 +101,21 @@ nsBMPDecoder::GetBitsPerPixel() const
 
 // Obtains the width from the internal BIH header
 PRInt32 
 nsBMPDecoder::GetWidth() const
 {
   return mBIH.width; 
 }
 
-// Obtains the height from the internal BIH header
+// Obtains the abs-value of the height from the internal BIH header
 PRInt32 
 nsBMPDecoder::GetHeight() const
 {
-  return mBIH.height; 
+  return abs(mBIH.height);
 }
 
 // Obtains the internal output image buffer
 PRUint32* 
 nsBMPDecoder::GetImageData() 
 {
   return mImageData;
 }
@@ -134,17 +134,17 @@ nsBMPDecoder::GetCompressedImageSize() c
   PRUint32 rowSize = (mBIH.bpp * mBIH.width + 7) / 8; // + 7 to round up
   // Pad to DWORD Boundary
   if (rowSize % 4) {
     rowSize += (4 - (rowSize % 4));
   }
 
   // The height should be the absolute value of what the height is in the BIH.
   // If positive the bitmap is stored bottom to top, otherwise top to bottom
-  PRInt32 pixelArraySize = rowSize * abs(mBIH.height); 
+  PRInt32 pixelArraySize = rowSize * GetHeight();
   return pixelArraySize;
 }
 
 // Obtains whether or not a BMP file had alpha data in its 4th byte
 // for 32BPP bitmaps.  Only use after the bitmap has been processed.
 bool 
 nsBMPDecoder::HasAlphaData() const 
 {
@@ -160,17 +160,17 @@ nsBMPDecoder::FinishInternal()
 
     // We should never make multiple frames
     NS_ABORT_IF_FALSE(GetFrameCount() <= 1, "Multiple BMP frames?");
 
     // Send notifications if appropriate
     if (!IsSizeDecode() && (GetFrameCount() == 1)) {
 
         // Invalidate
-        nsIntRect r(0, 0, mBIH.width, mBIH.height);
+        nsIntRect r(0, 0, mBIH.width, GetHeight());
         PostInvalidation(r);
 
         PostFrameStop();
         PostDecodeDone();
     }
 }
 
 // ----------------------------------------
@@ -268,17 +268,17 @@ nsBMPDecoder::WriteInternal(const char* 
         // BMPs with negative width are invalid
         // Reject extremely wide images to keep the math sane
         const PRInt32 k64KWidth = 0x0000FFFF;
         if (mBIH.width < 0 || mBIH.width > k64KWidth) {
             PostDataError();
             return;
         }
 
-        PRUint32 real_height = (mBIH.height > 0) ? mBIH.height : -mBIH.height;
+        PRUint32 real_height = GetHeight();
 
         // Post our size to the superclass
         PostSize(mBIH.width, real_height);
         if (HasError()) {
           // Setting the size lead to an error; this can happen when for example
           // a multipart channel sends an image of a different size.
           return;
         }
@@ -506,19 +506,22 @@ nsBMPDecoder::WriteInternal(const char* 
                             if (!mHaveAlphaData && p[3]) {
                               // Non-zero alpha byte detected! Clear previous
                               // pixels that we have already processed.
                               // This works because we know that if we 
                               // are reaching here then the alpha data in byte 
                               // 4 has been right all along.  And we know it
                               // has been set to 0 the whole time, so that 
                               // means that everything is transparent so far.
-                              memset(mImageData + (mCurLine - 1) * GetWidth(), 0, 
-                                     (GetHeight() - mCurLine + 1) * 
-                                     GetWidth() * sizeof(PRUint32));
+                              PRUint32* start = mImageData + GetWidth() * (mCurLine - 1);
+                              PRUint32 heightDifference = GetHeight() - mCurLine + 1;
+                              PRUint32 pixelCount = GetWidth() * heightDifference;
+
+                              memset(start, 0, pixelCount * sizeof(PRUint32));
+
                               mHaveAlphaData = true;
                             }
                             SetPixel(d, p[2], p[1], p[0], mHaveAlphaData ? p[3] : 0xFF);
                           } else {
                             SetPixel(d, p[2], p[1], p[0]);
                           }
                           p += 4;
                           --lpos;
--- a/image/decoders/nsBMPDecoder.h
+++ b/image/decoders/nsBMPDecoder.h
@@ -65,17 +65,17 @@ public:
     // Specifies whether or not the BMP file will contain alpha data
     // If set to true and the BMP is 32BPP, the alpha data will be
     // retrieved from the 4th byte of image data per pixel 
     void SetUseAlphaData(bool useAlphaData);
     // Obtains the bits per pixel from the internal BIH header
     PRInt32 GetBitsPerPixel() const;
     // Obtains the width from the internal BIH header
     PRInt32 GetWidth() const;
-    // Obtains the height from the internal BIH header
+    // Obtains the abs-value of the height from the internal BIH header
     PRInt32 GetHeight() const;
     // Obtains the internal output image buffer
     PRUint32* GetImageData();
     // Obtains the size of the compressed image resource
     PRInt32 GetCompressedImageSize() const;
     // Obtains whether or not a BMP file had alpha data in its 4th byte
     // for 32BPP bitmaps.  Only use after the bitmap has been processed.
     bool HasAlphaData() const;
@@ -100,17 +100,17 @@ private:
     PRUint32 mNumColors; ///< The number of used colors, i.e. the number of entries in mColors
     colorTable *mColors;
 
     bitFields mBitFields;
 
     PRUint32 *mImageData; ///< Pointer to the image data for the frame
     PRUint8 *mRow;      ///< Holds one raw line of the image
     PRUint32 mRowBytes; ///< How many bytes of the row were already received
-    PRInt32 mCurLine;   ///< Index of the line of the image that's currently being decoded
+    PRInt32 mCurLine;   ///< Index of the line of the image that's currently being decoded: [height,1]
     PRInt32 mOldLine;   ///< Previous index of the line 
     PRInt32 mCurPos;    ///< Index in the current line of the image
 
     ERLEState mState;   ///< Maintains the current state of the RLE decoding
     PRUint32 mStateData;///< Decoding information that is needed depending on mState
 
     /** Set mBFH from the raw data in mRawBuf, converting from little-endian
      * data to native data as necessary */