From c176fc94fe0b526db7c838196e26584b1428fc69 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 13 Oct 2015 21:20:10 -0700 Subject: [PATCH] Bug 1214072 (part 2) - Implement transparency properly for BMP images. r=seth. Currently we don't implement transparency at all in BMP images except for an odd-duck case of BMPs within ICOs. This patch does the following. - It implements transparency properly for 16bpp and 32bpp images via bitfield masking. (For 32bpp images this also requires handling colors via bitfield masking.) The patch maintains the existing BMP-within-ICO transparency handling. - It also reworks BitFields::Value::Set(). * It now works correctly if the run of 1s goes all the way to bit 31 (the old code didn't set mBitWidth). * If the mask is 0, will give an mRightShift of 0 (old code gave 32, and right-shifting by 32 is dodgy). * It's now easier to read. - It renames transparent.bmp as transparent-if-within-ico.bmp. Ironically enough this file currently uses BITFIELDS compression and is WinBMPv5 format, which means it contains well-specified alpha data. In order to use it to test the hacky BMP-within-ICO transparency scheme the patch changes it to be WinBMPv3 format with RGB compression (i.e. no compression). I left the now-excess bytes (including the bitfields) in the info header in place because that's allowed -- thanks to the start of the pixel data being specified by the |dataoffset| field -- they'll just be ignored. - It tweaks the naming of the relevant gtests and some of their finer details to work with the new way of doing things. This fixes all four remaining failures in bmpsuite. --- image/decoders/nsBMPDecoder.cpp | 201 +++++++++++++----- image/decoders/nsBMPDecoder.h | 71 ++++--- image/decoders/nsICODecoder.cpp | 8 +- image/test/gtest/Common.cpp | 12 +- image/test/gtest/TestMetadata.cpp | 32 +-- image/test/gtest/moz.build | 2 +- ...rent.bmp => transparent-if-within-ico.bmp} | Bin 4234 -> 4234 bytes .../test/reftest/bmp/bmpsuite/g/reftest.list | 4 +- .../test/reftest/bmp/bmpsuite/q/reftest.list | 12 +- 9 files changed, 229 insertions(+), 113 deletions(-) rename image/test/gtest/{transparent.bmp => transparent-if-within-ico.bmp} (98%) diff --git a/image/decoders/nsBMPDecoder.cpp b/image/decoders/nsBMPDecoder.cpp index 2b63c2a943e..4912dc5d3c7 100644 --- a/image/decoders/nsBMPDecoder.cpp +++ b/image/decoders/nsBMPDecoder.cpp @@ -60,6 +60,12 @@ // - It also added an optional color profile table after the pixel data (and // another optional gap). // +// WinBMPv3-ICO. This is a variant of WinBMPv3. +// - It's the BMP format used for BMP images within ICO files. +// - The only difference with WinBMPv3 is that if an image is 32bpp and has no +// compression, then instead of treating the pixel data as 0RGB it is treated +// as ARGB, but only if one or more of the A values are non-zero. +// // OS/2 VERSIONS OF THE BMP FORMAT // ------------------------------- // OS2-BMPv1. @@ -168,7 +174,9 @@ GetBMPLog() nsBMPDecoder::nsBMPDecoder(RasterImage* aImage) : Decoder(aImage) , mLexer(Transition::To(State::FILE_HEADER, FileHeader::LENGTH)) + , mIsWithinICO(false) , mMayHaveTransparency(false) + , mDoesHaveTransparency(false) , mNumColors(0) , mColors(nullptr) , mBytesPerColor(0) @@ -176,8 +184,6 @@ nsBMPDecoder::nsBMPDecoder(RasterImage* aImage) , mCurrentRow(0) , mCurrentPos(0) , mAbsoluteModeNumPixels(0) - , mUseAlphaData(false) - , mHaveAlphaData(false) { memset(&mBFH, 0, sizeof(mBFH)); memset(&mBIH, 0, sizeof(mBIH)); @@ -188,13 +194,6 @@ nsBMPDecoder::~nsBMPDecoder() delete[] mColors; } -// Sets whether or not the BMP will use alpha data. -void -nsBMPDecoder::SetUseAlphaData(bool useAlphaData) -{ - mUseAlphaData = useAlphaData; -} - // Obtains the bits per pixel from the internal BIH header. int32_t nsBMPDecoder::GetBitsPerPixel() const @@ -251,7 +250,8 @@ nsBMPDecoder::FinishInternal() nsIntRect r(0, 0, mBIH.width, GetHeight()); PostInvalidation(r); - if (mUseAlphaData && mHaveAlphaData) { + if (mDoesHaveTransparency) { + MOZ_ASSERT(mMayHaveTransparency); PostFrameStop(Opacity::SOME_TRANSPARENCY); } else { PostFrameStop(Opacity::OPAQUE); @@ -269,21 +269,39 @@ BitFields::Value::Set(uint32_t aMask) { mMask = aMask; + // Handle this exceptional case first. The chosen values don't matter + // (because a mask of zero will always give a value of zero) except that + // mBitWidth: + // - shouldn't be zero, because that would cause an infinite loop in Get(); + // - shouldn't be 5 or 8, because that could cause a false positive match in + // IsR5G5B5() or IsR8G8B8(). + if (mMask == 0x0) { + mRightShift = 0; + mBitWidth = 1; + return; + } + // Find the rightmost 1. - bool started = false; - mRightShift = mBitWidth = 0; - for (uint8_t pos = 0; pos <= 31; pos++) { - if (!started && (aMask & (1 << pos))) { - mRightShift = pos; - started = true; - } else if (started && !(aMask & (1 << pos))) { - mBitWidth = pos - mRightShift; + uint8_t i; + for (i = 0; i < 32; i++) { + if (mMask & (1 << i)) { break; } } + mRightShift = i; + + // Now find the leftmost 1 in the same run of 1s. (If there are multiple runs + // of 1s -- which isn't valid -- we'll behave as if only the lowest run was + // present, which seems reasonable.) + for (i = i + 1; i < 32; i++) { + if (!(mMask & (1 << i))) { + break; + } + } + mBitWidth = i - mRightShift; } -inline uint8_t +MOZ_ALWAYS_INLINE uint8_t BitFields::Value::Get(uint32_t aValue) const { // Extract the unscaled value. @@ -315,6 +333,16 @@ BitFields::Value::Get(uint32_t aValue) const return v2; } +MOZ_ALWAYS_INLINE uint8_t +BitFields::Value::GetAlpha(uint32_t aValue, bool& aHasAlphaOut) const +{ + if (mMask == 0x0) { + return 0xff; + } + aHasAlphaOut = true; + return Get(aValue); +} + MOZ_ALWAYS_INLINE uint8_t BitFields::Value::Get5(uint32_t aValue) const { @@ -323,6 +351,14 @@ BitFields::Value::Get5(uint32_t aValue) const return (v << 3u) | (v >> 2u); } +MOZ_ALWAYS_INLINE uint8_t +BitFields::Value::Get8(uint32_t aValue) const +{ + MOZ_ASSERT(mBitWidth == 8); + uint32_t v = (aValue & mMask) >> mRightShift; + return v; +} + void BitFields::SetR5G5B5() { @@ -331,12 +367,30 @@ BitFields::SetR5G5B5() mBlue.Set(0x001f); } +void +BitFields::SetR8G8B8() +{ + mRed.Set(0xff0000); + mGreen.Set(0xff00); + mBlue.Set(0x00ff); +} + bool BitFields::IsR5G5B5() const { return mRed.mBitWidth == 5 && mGreen.mBitWidth == 5 && - mBlue.mBitWidth == 5; + mBlue.mBitWidth == 5 && + mAlpha.mMask == 0x0; +} + +bool +BitFields::IsR8G8B8() const +{ + return mRed.mBitWidth == 8 && + mGreen.mBitWidth == 8 && + mBlue.mBitWidth == 8 && + mAlpha.mMask == 0x0; } uint32_t* @@ -459,6 +513,9 @@ nsBMPDecoder::ReadInfoHeaderSize(const char* aData, size_t aLength) PostDataError(); return Transition::Terminate(State::FAILURE); } + // ICO BMPs must have a WinVMPv3 header. nsICODecoder should have already + // terminated decoding if this isn't the case. + MOZ_ASSERT_IF(mIsWithinICO, mBIH.bihsize == InfoHeaderLength::WIN_V3); return Transition::To(State::INFO_HEADER_REST, mBIH.bihsize - BIHSIZE_FIELD_LENGTH); @@ -542,23 +599,13 @@ nsBMPDecoder::ReadInfoHeaderRest(const char* aData, size_t aLength) mPixelRowSize += 4 - surplus; } - // We treat BMPs as transparent if they're 32bpp and alpha is enabled, but - // also if they use RLE encoding, because the 'delta' mode can skip pixels - // and cause implicit transparency. - mMayHaveTransparency = (mBIH.compression == Compression::RLE8) || - (mBIH.compression == Compression::RLE4) || - (mBIH.bpp == 32 && mUseAlphaData); - if (mMayHaveTransparency) { - PostHasTransparency(); - } - size_t bitFieldsLengthStillToRead = 0; if (mBIH.compression == Compression::BITFIELDS) { // Need to read bitfields. if (mBIH.bihsize >= InfoHeaderLength::WIN_V4) { // Bitfields are present in the info header, so we can read them // immediately. - mBitFields.ReadFromHeader(aData + 36); + mBitFields.ReadFromHeader(aData + 36, /* aReadAlpha = */ true); } else { // Bitfields are present after the info header, so we will read them in // ReadBitfields(). @@ -567,17 +614,23 @@ nsBMPDecoder::ReadInfoHeaderRest(const char* aData, size_t aLength) } else if (mBIH.bpp == 16) { // No bitfields specified; use the default 5-5-5 values. mBitFields.SetR5G5B5(); + } else if (mBIH.bpp == 32) { + // No bitfields specified; use the default 8-8-8 values. + mBitFields.SetR8G8B8(); } return Transition::To(State::BITFIELDS, bitFieldsLengthStillToRead); } void -BitFields::ReadFromHeader(const char* aData) +BitFields::ReadFromHeader(const char* aData, bool aReadAlpha) { mRed.Set (LittleEndian::readUint32(aData + 0)); mGreen.Set(LittleEndian::readUint32(aData + 4)); mBlue.Set (LittleEndian::readUint32(aData + 8)); + if (aReadAlpha) { + mAlpha.Set(LittleEndian::readUint32(aData + 12)); + } } LexerTransition @@ -588,7 +641,19 @@ nsBMPDecoder::ReadBitfields(const char* aData, size_t aLength) // If aLength is zero there are no bitfields to read, or we already read them // in ReadInfoHeader(). if (aLength != 0) { - mBitFields.ReadFromHeader(aData); + mBitFields.ReadFromHeader(aData, /* aReadAlpha = */ false); + } + + // Note that RLE-encoded BMPs might be transparent because the 'delta' mode + // can skip pixels and cause implicit transparency. + mMayHaveTransparency = + (mBIH.compression == Compression::RGB && mIsWithinICO && mBIH.bpp == 32) || + mBIH.compression == Compression::RLE8 || + mBIH.compression == Compression::RLE4 || + (mBIH.compression == Compression::BITFIELDS && + mBitFields.mAlpha.IsPresent()); + if (mMayHaveTransparency) { + PostHasTransparency(); } // We've now read all the headers. If we're doing a metadata decode, we're @@ -726,14 +791,20 @@ nsBMPDecoder::ReadPixelRow(const char* aData) src += 2; } } else { + bool anyHasAlpha = false; while (lpos > 0) { uint16_t val = LittleEndian::readUint16(src); SetPixel(dst, mBitFields.mRed.Get(val), mBitFields.mGreen.Get(val), - mBitFields.mBlue.Get(val)); + mBitFields.mBlue.Get(val), + mBitFields.mAlpha.GetAlpha(val, anyHasAlpha)); --lpos; src += 2; } + if (anyHasAlpha) { + MOZ_ASSERT(mMayHaveTransparency); + mDoesHaveTransparency = true; + } } break; @@ -746,18 +817,53 @@ nsBMPDecoder::ReadPixelRow(const char* aData) break; case 32: - while (lpos > 0) { - if (mUseAlphaData) { - if (MOZ_UNLIKELY(!mHaveAlphaData && src[3])) { - PostHasTransparency(); - mHaveAlphaData = true; + if (mBIH.compression == Compression::RGB && mIsWithinICO && + mBIH.bpp == 32) { + // This is a special case only used for 32bpp WinBMPv3-ICO files, which + // could be in either 0RGB or ARGB format. + 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) { + MOZ_ASSERT(mMayHaveTransparency); + mDoesHaveTransparency = true; } SetPixel(dst, src[2], src[1], src[0], src[3]); - } else { - SetPixel(dst, src[2], src[1], src[0]); + 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), + mBitFields.mGreen.Get8(val), + mBitFields.mBlue.Get8(val)); + --lpos; + src += 4; + } + } else { + bool anyHasAlpha = false; + while (lpos > 0) { + uint32_t val = LittleEndian::readUint32(src); + SetPixel(dst, mBitFields.mRed.Get(val), + mBitFields.mGreen.Get(val), + mBitFields.mBlue.Get(val), + mBitFields.mAlpha.GetAlpha(val, anyHasAlpha)); + --lpos; + src += 4; + } + if (anyHasAlpha) { + MOZ_ASSERT(mMayHaveTransparency); + mDoesHaveTransparency = true; } - --lpos; - src += 4; } break; @@ -840,13 +946,10 @@ nsBMPDecoder::ReadRLESegment(const char* aData) LexerTransition nsBMPDecoder::ReadRLEDelta(const char* aData) { - // Delta encoding makes it possible to skip pixels making the image + // Delta encoding makes it possible to skip pixels making part of the image // transparent. - if (MOZ_UNLIKELY(!mHaveAlphaData)) { - PostHasTransparency(); - mHaveAlphaData = true; - } - mUseAlphaData = mHaveAlphaData = true; + MOZ_ASSERT(mMayHaveTransparency); + mDoesHaveTransparency = true; if (mDownscaler) { // Clear the skipped pixels. (This clears to the end of the row, diff --git a/image/decoders/nsBMPDecoder.h b/image/decoders/nsBMPDecoder.h index 5c0c3392d72..4ed4998e35e 100644 --- a/image/decoders/nsBMPDecoder.h +++ b/image/decoders/nsBMPDecoder.h @@ -38,12 +38,26 @@ class BitFields { void Set(uint32_t aMask); public: + Value() + { + mMask = 0; + mRightShift = 0; + mBitWidth = 0; + } + + /// Returns true if this channel is used. Only used for alpha. + bool IsPresent() const { return mMask != 0x0; } + /// Extracts the single color value from the multi-color value. uint8_t Get(uint32_t aVal) const; - /// Specialized version of Get() for the case where the bit-width is 5. - /// (It will assert if called and the bit-width is not 5.) + /// Like Get(), but specially for alpha. + uint8_t GetAlpha(uint32_t aVal, bool& aHasAlphaOut) const; + + /// Specialized versions of Get() for when the bit-width is 5 or 8. + /// (They will assert if called and the bit-width is not 5 or 8.) uint8_t Get5(uint32_t aVal) const; + uint8_t Get8(uint32_t aVal) const; }; public: @@ -51,15 +65,23 @@ public: Value mRed; Value mGreen; Value mBlue; + Value mAlpha; /// Set bitfields to the standard 5-5-5 16bpp values. void SetR5G5B5(); + /// Set bitfields to the standard 8-8-8 32bpp values. + void SetR8G8B8(); + /// Test if bitfields have the standard 5-5-5 16bpp values. bool IsR5G5B5() const; - /// Read the bitfields from a header. - void ReadFromHeader(const char* aData); + /// Test if bitfields have the standard 8-8-8 32bpp values. + bool IsR8G8B8() const; + + /// Read the bitfields from a header. The reading of the alpha mask is + /// optional. + void ReadFromHeader(const char* aData, bool aReadAlpha); /// Length of the bitfields structure in the BMP file. static const size_t LENGTH = 12; @@ -76,11 +98,6 @@ class nsBMPDecoder : public Decoder public: ~nsBMPDecoder(); - /// 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. int32_t GetBitsPerPixel() const; @@ -97,12 +114,19 @@ public: /// Obtains the size of the compressed image resource. int32_t GetCompressedImageSize() const; - /// Obtains whether or not a BMP file had alpha data in its 4th byte for 32BPP - /// bitmaps. Use only after the bitmap has been processed. - bool HasAlphaData() const { return mHaveAlphaData; } + /// Mark this BMP as being within an ICO file. + void SetIsWithinICO() { mIsWithinICO = true; } - /// Marks this BMP as having alpha data (due to e.g. an ICO alpha mask). - void SetHasAlphaData() { mHaveAlphaData = true; } + /// Did the BMP file have alpha data of any kind? (Only use this after the + /// bitmap has been fully decoded.) + bool HasTransparency() const { return mDoesHaveTransparency; } + + /// Force transparency from outside. (Used by the ICO decoder.) + void SetHasTransparency() + { + mMayHaveTransparency = true; + mDoesHaveTransparency = true; + } virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override; @@ -151,12 +175,19 @@ private: bmp::FileHeader mBFH; bmp::V5InfoHeader mBIH; + // If the BMP is within an ICO file our treatment of it differs slightly. + bool mIsWithinICO; + bmp::BitFields mBitFields; // Might the image have transparency? Determined from the headers during // metadata decode. (Does not guarantee the image actually has transparency.) bool mMayHaveTransparency; + // Does the image have transparency? Determined during full decoding, so only + // use this after that has been completed. + bool mDoesHaveTransparency; + uint32_t mNumColors; // The number of used colors, i.e. the number of // entries in mColors, if it's present. bmp::ColorTableEntry* mColors; // The color table, if it's present. @@ -175,18 +206,6 @@ private: // Only used in RLE_ABSOLUTE state: the number of pixels to read. uint32_t mAbsoluteModeNumPixels; - - // Stores whether the image data may store alpha data, or if the alpha data - // is unspecified and filled with a padding byte of 0. When a 32BPP bitmap - // is stored in an ICO or CUR file, its 4th byte is used for alpha - // transparency. When it is stored in a BMP, its 4th byte is reserved and - // is always 0. Reference: - // http://en.wikipedia.org/wiki/ICO_(file_format)#cite_note-9 - // Bitmaps where the alpha bytes are all 0 should be fully visible. - bool mUseAlphaData; - - // Whether the 4th byte alpha data was found to be non zero and hence used. - bool mHaveAlphaData; }; } // namespace image diff --git a/image/decoders/nsICODecoder.cpp b/image/decoders/nsICODecoder.cpp index ad49c4089dd..982e941f855 100644 --- a/image/decoders/nsICODecoder.cpp +++ b/image/decoders/nsICODecoder.cpp @@ -393,7 +393,7 @@ nsICODecoder::SniffResource(const char* aData) // is the AND mask, which isn't present in standalone BMPs. nsBMPDecoder* bmpDecoder = new nsBMPDecoder(mImage); mContainedDecoder = bmpDecoder; - bmpDecoder->SetUseAlphaData(true); + bmpDecoder->SetIsWithinICO(); mContainedDecoder->SetMetadataDecode(IsMetadataDecode()); mContainedDecoder->SetDecoderFlags(GetDecoderFlags()); mContainedDecoder->SetSurfaceFlags(GetSurfaceFlags()); @@ -525,9 +525,9 @@ nsICODecoder::PrepareForMask() MOZ_ASSERT(bmpLengthWithHeader < mDirEntry.mBytesInRes); uint32_t maskLength = mDirEntry.mBytesInRes - bmpLengthWithHeader; - // If we have a 32-bpp BMP with alpha data, we ignore the AND mask. We can + // If the BMP provides its own transparency, we ignore the AND mask. We can // also obviously ignore it if the image has zero width or zero height. - if ((bmpDecoder->GetBitsPerPixel() == 32 && bmpDecoder->HasAlphaData()) || + if (bmpDecoder->HasTransparency() || GetRealWidth() == 0 || GetRealHeight() == 0) { return Transition::ToUnbuffered(ICOState::FINISHED_RESOURCE, ICOState::SKIP_MASK, @@ -658,7 +658,7 @@ nsICODecoder::FinishMask() RefPtr bmpDecoder = static_cast(mContainedDecoder.get()); - bmpDecoder->SetHasAlphaData(); + bmpDecoder->SetHasTransparency(); } return Transition::To(ICOState::FINISHED_RESOURCE, 0); diff --git a/image/test/gtest/Common.cpp b/image/test/gtest/Common.cpp index 80a52d30a43..736b59c4b39 100644 --- a/image/test/gtest/Common.cpp +++ b/image/test/gtest/Common.cpp @@ -194,13 +194,13 @@ ImageTestCase FirstFramePaddingGIFTestCase() TEST_CASE_IS_TRANSPARENT); } -ImageTestCase TransparentBMPWhenBMPAlphaEnabledTestCase() +ImageTestCase TransparentIfWithinICOBMPTestCase(TestCaseFlags aFlags) { - // Note that we only decode this test case as transparent when the BMP decoder - // is set to use alpha data. (That's not the default, which is why it's not marked - // TEST_CASE_IS_TRANSPARENT; tests that want to treat this testcase as - // transparent need to handle this case manually.) - return ImageTestCase("transparent.bmp", "image/bmp", IntSize(32, 32)); + // This is a BMP that is only transparent when decoded as if it is within an + // ICO file. (Note: aFlags needs to be set to TEST_CASE_DEFAULT_FLAGS or + // TEST_CASE_IS_TRANSPARENT accordingly.) + return ImageTestCase("transparent-if-within-ico.bmp", "image/bmp", + IntSize(32, 32), aFlags); } ImageTestCase RLE4BMPTestCase() diff --git a/image/test/gtest/TestMetadata.cpp b/image/test/gtest/TestMetadata.cpp index 7c7261880a5..57433e24d88 100644 --- a/image/test/gtest/TestMetadata.cpp +++ b/image/test/gtest/TestMetadata.cpp @@ -38,15 +38,15 @@ TEST(ImageMetadata, ImageModuleAvailable) EXPECT_TRUE(imgTools != nullptr); } -enum class BMPAlpha +enum class BMPWithinICO { - DISABLED, - ENABLED + NO, + YES }; static void CheckMetadata(const ImageTestCase& aTestCase, - BMPAlpha aBMPAlpha = BMPAlpha::DISABLED) + BMPWithinICO aBMPWithinICO = BMPWithinICO::NO) { nsCOMPtr inputStream = LoadFile(aTestCase.mPath); ASSERT_TRUE(inputStream != nullptr); @@ -70,13 +70,13 @@ CheckMetadata(const ImageTestCase& aTestCase, DecoderFactory::CreateAnonymousMetadataDecoder(decoderType, sourceBuffer); ASSERT_TRUE(decoder != nullptr); - if (aBMPAlpha == BMPAlpha::ENABLED) { - static_cast(decoder.get())->SetUseAlphaData(true); + if (aBMPWithinICO == BMPWithinICO::YES) { + static_cast(decoder.get())->SetIsWithinICO(); } // Run the metadata decoder synchronously. decoder->Decode(); - + // Ensure that the metadata decoder didn't make progress it shouldn't have // (which would indicate that it decoded past the header of the image). Progress metadataProgress = decoder->TakeProgress(); @@ -100,7 +100,7 @@ CheckMetadata(const ImageTestCase& aTestCase, EXPECT_EQ(aTestCase.mSize.width, metadataSize.width); EXPECT_EQ(aTestCase.mSize.height, metadataSize.height); - bool expectTransparency = aBMPAlpha == BMPAlpha::ENABLED + bool expectTransparency = aBMPWithinICO == BMPWithinICO::YES ? true : bool(aTestCase.mFlags & TEST_CASE_IS_TRANSPARENT); EXPECT_EQ(expectTransparency, bool(metadataProgress & FLAG_HAS_TRANSPARENCY)); @@ -114,13 +114,13 @@ CheckMetadata(const ImageTestCase& aTestCase, DefaultSurfaceFlags()); ASSERT_TRUE(decoder != nullptr); - if (aBMPAlpha == BMPAlpha::ENABLED) { - static_cast(decoder.get())->SetUseAlphaData(true); + if (aBMPWithinICO == BMPWithinICO::YES) { + static_cast(decoder.get())->SetIsWithinICO(); } // Run the full decoder synchronously. decoder->Decode(); - + EXPECT_TRUE(decoder->GetDecodeDone() && !decoder->HasError()); Progress fullProgress = decoder->TakeProgress(); @@ -164,14 +164,16 @@ TEST(ImageMetadata, FirstFramePaddingGIF) CheckMetadata(FirstFramePaddingGIFTestCase()); } -TEST(ImageMetadata, TransparentBMPWithBMPAlphaOff) +TEST(ImageMetadata, TransparentIfWithinICOBMPNotWithinICO) { - CheckMetadata(TransparentBMPWhenBMPAlphaEnabledTestCase(), BMPAlpha::ENABLED); + CheckMetadata(TransparentIfWithinICOBMPTestCase(TEST_CASE_DEFAULT_FLAGS), + BMPWithinICO::NO); } -TEST(ImageMetadata, TransparentBMPWithBMPAlphaOn) +TEST(ImageMetadata, TransparentIfWithinICOBMPWithinICO) { - CheckMetadata(TransparentBMPWhenBMPAlphaEnabledTestCase(), BMPAlpha::ENABLED); + CheckMetadata(TransparentIfWithinICOBMPTestCase(TEST_CASE_IS_TRANSPARENT), + BMPWithinICO::YES); } TEST(ImageMetadata, RLE4BMP) { CheckMetadata(RLE4BMPTestCase()); } diff --git a/image/test/gtest/moz.build b/image/test/gtest/moz.build index 90c23bf6f59..dd23f42aaa2 100644 --- a/image/test/gtest/moz.build +++ b/image/test/gtest/moz.build @@ -29,7 +29,7 @@ TEST_HARNESS_FILES.gtest += [ 'no-frame-delay.gif', 'rle4.bmp', 'rle8.bmp', - 'transparent.bmp', + 'transparent-if-within-ico.bmp', 'transparent.gif', 'transparent.png', ] diff --git a/image/test/gtest/transparent.bmp b/image/test/gtest/transparent-if-within-ico.bmp similarity index 98% rename from image/test/gtest/transparent.bmp rename to image/test/gtest/transparent-if-within-ico.bmp index c3ee2289598752d21f1f922520f5430f689382f3..4dc04c181ba7d4ca9ac41fcb670f6f5fa3ca37dd 100644 GIT binary patch delta 40 hcmeBD>{66>^6e5}0D~?dsR6_aP|OHoZIsUz004N{1pNR2 delta 40 icmeBD>{66>^6e5}0D~?dSp&ohP|OHqF>jR5761T$%LP;b diff --git a/image/test/reftest/bmp/bmpsuite/g/reftest.list b/image/test/reftest/bmp/bmpsuite/g/reftest.list index e1ed77b5f05..9715b1ac82a 100644 --- a/image/test/reftest/bmp/bmpsuite/g/reftest.list +++ b/image/test/reftest/bmp/bmpsuite/g/reftest.list @@ -108,7 +108,5 @@ fuzzy(1,1516) == rgb16.bmp rgb16.png # "A 32-bit image with a BITFIELDS segment. As usual, there are 8 bits per color # channel, and 8 unused bits. But the color channels are in an unusual order, # so the viewer must read the BITFIELDS, and not just guess." -# [XXX: we get this completely wrong because we don't handle BITFIELDS at all -# in 32bpp BMPs. Chromium gets this right.] -fails == rgb32bf.bmp rgb24.png +== rgb32bf.bmp rgb24.png diff --git a/image/test/reftest/bmp/bmpsuite/q/reftest.list b/image/test/reftest/bmp/bmpsuite/q/reftest.list index 4a6d882cc92..80da580cc8f 100644 --- a/image/test/reftest/bmp/bmpsuite/q/reftest.list +++ b/image/test/reftest/bmp/bmpsuite/q/reftest.list @@ -71,9 +71,7 @@ fuzzy(1,899) == pal8os2v2-16.bmp pal8.png # "A 16-bit image with an alpha channel. There are 4 bits for each color # channel, and 4 bits for the alpha channel. It’s not clear if this is valid, # but I can’t find anything that suggests it isn’t." -# [XXX: we don't even try to do transparency for 16bpp. Chromium gets the -# transparency right.] -fails == rgba16-4444.bmp rgba16-4444.png +== rgba16-4444.bmp rgba16-4444.png # BMP: bihsize=40, 127 x 64, bpp=24, compression=0, colors=300 # "A 24-bit image, with a palette containing 300 colors. The fact that the @@ -114,18 +112,14 @@ fails == rgba16-4444.bmp rgba16-4444.png # BMP: bihsize=40, 127 x 64, bpp=32, compression=3, colors=0 # "A 32 bits/pixel image, with all 32 bits used: 11 each for red and green, and # 10 for blue. As far as I know, this is perfectly valid, but it is unusual." -# [XXX: we get this completely wrong because we don't handle BITFIELDS at all -# in 32bpp BMPs. Chromium gets this right.] -fails == rgb32-111110.bmp rgb24.png +fuzzy(1,1408) == rgb32-111110.bmp rgb24.png # BMP: bihsize=124, 127 x 64, bpp=32, compression=3, colors=0 # "A BMP with an alpha channel. Transparency is barely documented, so it’s # possible that this file is not correctly formed. The color channels are in an # unusual order, to prevent viewers from passing this test by making a lucky # guess." -# [XXX: we get this completely wrong because we don't handle BITFIELDS at all -# in 32bpp BMPs, especially not with alpha. Chromium gets this right.] -fails == rgba32.bmp rgba32.png +== rgba32.bmp rgba32.png # BMP: bihsize=40, 127 x 64, bpp=32, compression=6, colors=0 # "An image of type BI_ALPHABITFIELDS. Supposedly, this was used on Windows CE.