Bug 775793 - Clarify BMP decoding for unexpected alpha pixels - r=bbondy
authorJeff Gilbert <jgilbert@mozilla.com>
Mon, 23 Jul 2012 13:20:57 -0700
changeset 100213 673c2c03067a31c2f9044f80d0204316efb98b8d
parent 100212 e6fd182462400e161f474126d86c2836d2705301
child 100214 fbe21dc7e1d9db4a6882d997af1a2b78b86885f0
push id12379
push userjgilbert@mozilla.com
push dateMon, 23 Jul 2012 20:22:10 +0000
treeherdermozilla-inbound@673c2c03067a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbbondy
bugs775793
milestone17.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 775793 - Clarify BMP decoding for unexpected alpha pixels - r=bbondy
image/decoders/nsBMPDecoder.cpp
image/decoders/nsBMPDecoder.h
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -66,21 +66,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;
 }
@@ -99,17 +99,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 
 {
@@ -125,17 +125,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();
     }
 }
 
 // ----------------------------------------
@@ -233,17 +233,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 led to an error.
           return;
         }
 
@@ -475,19 +475,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
@@ -31,17 +31,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;
@@ -66,17 +66,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 */