Bug 1220021 (part 1) - Don't treat 0RGB ICO files as transparent. r=seth, a=lizzard
authorNicholas Nethercote <nnethercote@mozilla.com>
Mon, 23 Nov 2015 15:32:39 -0800
changeset 305686 9bb2dd1ff7f649fa0c760cf8f593fe05803a4cca
parent 305685 221ed303e8f153f8eaa7bf890ab94f9438c727dd
child 305687 77de40d65ca2359f1dfe7f23640e58866a654711
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseth, lizzard
bugs1220021, 1062066
milestone44.0a2
Bug 1220021 (part 1) - Don't treat 0RGB ICO files as transparent. r=seth, a=lizzard This approach is much the same as the one we had before bug 1062066 caused the regression.
image/decoders/nsBMPDecoder.cpp
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -797,32 +797,60 @@ nsBMPDecoder::ReadPixelRow(const char* a
         src += 3;
       }
       break;
 
     case 32:
       if (mH.mCompression == Compression::RGB && mIsWithinICO &&
           mH.mBpp == 32) {
         // This is a special case only used for 32bpp WinBMPv3-ICO files, which
-        // could be in either 0RGB or ARGB format.
+        // could be in either 0RGB or ARGB format. We start by assuming it's
+        // an 0RGB image. If we hit a non-zero alpha value, then we know it's
+        // actually an ARGB image, and change tack accordingly.
+        // (Note: a fully-transparent ARGB image is indistinguishable from a
+        // 0RGB image, and we will render such an image as a 0RGB image, i.e.
+        // opaquely. This is unlikely to be a problem in practice.)
         while (lpos > 0) {
-          // If src[3] is zero, we can't tell at this point if the image is
-          // 0RGB or ARGB. So we just use 0 value as-is. If the image is 0RGB
-          // then mDoesHaveTransparency will be false at the end, we'll treat
-          // the image as opaque, and the 0 alpha values will be ignored. If
-          // the image is ARGB then mDoesHaveTransparency will be true at the
-          // end and we'll treat the image as non-opaque. (Note: a
-          // fully-transparent ARGB image is indistinguishable from a 0RGB
-          // image, and we will render such an image as a 0RGB image, i.e.
-          // opaquely. This is unlikely to be a problem in practice.)
-          if (src[3] != 0) {
+          if (!mDoesHaveTransparency && src[3] != 0) {
+            // Up until now this looked like an 0RGB image, but we now know
+            // it's actually an ARGB image. Which means every pixel we've seen
+            // so far has been fully transparent. So we go back and redo them.
+
+            // Tell the Downscaler to go back to the start.
+            if (mDownscaler) {
+              mDownscaler->ResetForNextProgressivePass();
+            }
+
+            // Redo the complete rows we've already done.
+            MOZ_ASSERT(mCurrentPos == 0);
+            int32_t currentRow = mCurrentRow;
+            mCurrentRow = AbsoluteHeight();
+            while (mCurrentRow > currentRow) {
+              dst = RowBuffer();
+              for (int32_t i = 0; i < mH.mWidth; i++) {
+                SetPixel(dst, 0, 0, 0, 0);
+              }
+              FinishRow();
+            }
+
+            // Redo the part of this row we've already done.
+            dst = RowBuffer();
+            int32_t n = mH.mWidth - lpos;
+            for (int32_t i = 0; i < n; i++) {
+              SetPixel(dst, 0, 0, 0, 0);
+            }
+
             MOZ_ASSERT(mMayHaveTransparency);
             mDoesHaveTransparency = true;
           }
-          SetPixel(dst, src[2], src[1], src[0], src[3]);
+
+          // If mDoesHaveTransparency is false, treat this as an 0RGB image.
+          // Otherwise, treat this as an ARGB image.
+          SetPixel(dst, src[2], src[1], src[0],
+                   mDoesHaveTransparency ? src[3] : 0xff);
           src += 4;
           --lpos;
         }
       } else if (mBitFields.IsR8G8B8()) {
         // Specialize this common case.
         while (lpos > 0) {
           uint32_t val = LittleEndian::readUint32(src);
           SetPixel(dst, mBitFields.mRed.Get8(val),