Bug 1504237 - Ensure partial animated frames always use BGRA instead of BGRX. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 02 Nov 2018 15:28:17 -0400
changeset 500768 5fb87d2312ea
parent 500767 719fa010f362
child 500769 7339507cb663
child 500772 c5fa6b68042d
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1504237
milestone65.0a1
first release with
nightly linux32
5fb87d2312ea / 65.0a1 / 20181104100118 / files
nightly linux64
5fb87d2312ea / 65.0a1 / 20181104100118 / files
nightly mac
5fb87d2312ea / 65.0a1 / 20181104100118 / files
nightly win32
5fb87d2312ea / 65.0a1 / 20181104100118 / files
nightly win64
5fb87d2312ea / 65.0a1 / 20181104100118 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1504237 - Ensure partial animated frames always use BGRA instead of BGRX. r=tnikkel For decoders which produce unpaletted partial frames (APNG, WebP), the surface format should always be BGRA. These frames while partial, are the same size as the output size of the animated image. When FrameAnimator performs the blend with the compositing frame, it expects all pixels we don't care about to be set to fully transparent. If it is BGRX, they will be set to solid white instead. Differential Revision: https://phabricator.services.mozilla.com/D10753
image/decoders/nsWebPDecoder.cpp
image/test/gtest/Common.cpp
image/test/gtest/Common.h
image/test/gtest/TestFrameAnimator.cpp
image/test/gtest/blend.gif
image/test/gtest/blend.png
image/test/gtest/blend.webp
image/test/gtest/moz.build
--- a/image/decoders/nsWebPDecoder.cpp
+++ b/image/decoders/nsWebPDecoder.cpp
@@ -527,17 +527,18 @@ nsWebPDecoder::ReadMultiple(WebPDemuxer*
       case WEBP_MUX_DISPOSE_BACKGROUND:
         mDisposal = DisposalMethod::CLEAR;
         break;
       default:
         MOZ_ASSERT_UNREACHABLE("Unhandled dispose method");
         break;
     }
 
-    mFormat = iter.has_alpha ? SurfaceFormat::B8G8R8A8 : SurfaceFormat::B8G8R8X8;
+    mFormat = iter.has_alpha || mCurrentFrame > 0 ? SurfaceFormat::B8G8R8A8
+                                                  : SurfaceFormat::B8G8R8X8;
     mTimeout = FrameTimeout::FromRawMilliseconds(iter.duration);
     nsIntRect frameRect(iter.x_offset, iter.y_offset, iter.width, iter.height);
 
     rv = ReadSingle(iter.fragment.bytes, iter.fragment.size, frameRect);
     complete = complete && !WebPDemuxNextFrame(&iter);
     WebPDemuxReleaseIterator(&iter);
   }
 
--- a/image/test/gtest/Common.cpp
+++ b/image/test/gtest/Common.cpp
@@ -331,54 +331,67 @@ AssertCorrectPipelineFinalState(SurfaceF
 
 void
 CheckGeneratedImage(Decoder* aDecoder,
                     const IntRect& aRect,
                     uint8_t aFuzz /* = 0 */)
 {
   RawAccessFrameRef currentFrame = aDecoder->GetCurrentFrameRef();
   RefPtr<SourceSurface> surface = currentFrame->GetSourceSurface();
-  const IntSize surfaceSize = surface->GetSize();
+  CheckGeneratedSurface(surface, aRect,
+                        BGRAColor::Green(),
+                        BGRAColor::Transparent(),
+                        aFuzz);
+}
+
+void
+CheckGeneratedSurface(SourceSurface* aSurface,
+                      const IntRect& aRect,
+                      const BGRAColor& aInnerColor,
+                      const BGRAColor& aOuterColor,
+                      uint8_t aFuzz /* = 0 */)
+{
+  const IntSize surfaceSize = aSurface->GetSize();
 
   // This diagram shows how the surface is divided into regions that the code
   // below tests for the correct content. The output rect is the bounds of the
   // region labeled 'C'.
   //
   // +---------------------------+
   // |             A             |
   // +---------+--------+--------+
   // |    B    |   C    |   D    |
   // +---------+--------+--------+
   // |             E             |
   // +---------------------------+
 
-  // Check that the output rect itself is green. (Region 'C'.)
-  EXPECT_TRUE(RectIsSolidColor(surface, aRect, BGRAColor::Green(), aFuzz));
+  // Check that the output rect itself is the inner color. (Region 'C'.)
+  EXPECT_TRUE(RectIsSolidColor(aSurface, aRect, aInnerColor, aFuzz));
 
-  // Check that the area above the output rect is transparent. (Region 'A'.)
-  EXPECT_TRUE(RectIsSolidColor(surface,
+  // Check that the area above the output rect is the outer color. (Region 'A'.)
+  EXPECT_TRUE(RectIsSolidColor(aSurface,
                                IntRect(0, 0, surfaceSize.width, aRect.Y()),
-                               BGRAColor::Transparent(), aFuzz));
+                               aOuterColor, aFuzz));
 
-  // Check that the area to the left of the output rect is transparent. (Region 'B'.)
-  EXPECT_TRUE(RectIsSolidColor(surface,
+  // Check that the area to the left of the output rect is the outer color. (Region 'B'.)
+  EXPECT_TRUE(RectIsSolidColor(aSurface,
                                IntRect(0, aRect.Y(), aRect.X(), aRect.YMost()),
-                               BGRAColor::Transparent(), aFuzz));
+                               aOuterColor, aFuzz));
 
-  // Check that the area to the right of the output rect is transparent. (Region 'D'.)
+  // Check that the area to the right of the output rect is the outer color. (Region 'D'.)
   const int32_t widthOnRight = surfaceSize.width - aRect.XMost();
-  EXPECT_TRUE(RectIsSolidColor(surface,
+  EXPECT_TRUE(RectIsSolidColor(aSurface,
                                IntRect(aRect.XMost(), aRect.Y(), widthOnRight, aRect.YMost()),
-                               BGRAColor::Transparent(), aFuzz));
+                               aOuterColor, aFuzz));
 
-  // Check that the area below the output rect is transparent. (Region 'E'.)
+  // Check that the area below the output rect is the outer color. (Region 'E'.)
   const int32_t heightBelow = surfaceSize.height - aRect.YMost();
-  EXPECT_TRUE(RectIsSolidColor(surface,
+  EXPECT_TRUE(RectIsSolidColor(aSurface,
                                IntRect(0, aRect.YMost(), surfaceSize.width, heightBelow),
-                               BGRAColor::Transparent(), aFuzz));
+                               aOuterColor, aFuzz));
 }
 
 void
 CheckGeneratedPalettedImage(Decoder* aDecoder, const IntRect& aRect)
 {
   RawAccessFrameRef currentFrame = aDecoder->GetCurrentFrameRef();
   IntSize imageSize = currentFrame->GetImageSize();
 
@@ -585,16 +598,34 @@ ImageTestCase GreenFirstFrameAnimatedPNG
 }
 
 ImageTestCase GreenFirstFrameAnimatedWebPTestCase()
 {
   return ImageTestCase("first-frame-green.webp", "image/webp", IntSize(100, 100),
                        TEST_CASE_IS_ANIMATED);
 }
 
+ImageTestCase BlendAnimatedGIFTestCase()
+{
+  return ImageTestCase("blend.gif", "image/gif", IntSize(100, 100),
+                       TEST_CASE_IS_ANIMATED);
+}
+
+ImageTestCase BlendAnimatedPNGTestCase()
+{
+  return ImageTestCase("blend.png", "image/png", IntSize(100, 100),
+                       TEST_CASE_IS_TRANSPARENT | TEST_CASE_IS_ANIMATED);
+}
+
+ImageTestCase BlendAnimatedWebPTestCase()
+{
+  return ImageTestCase("blend.webp", "image/webp", IntSize(100, 100),
+                       TEST_CASE_IS_TRANSPARENT | TEST_CASE_IS_ANIMATED);
+}
+
 ImageTestCase CorruptTestCase()
 {
   return ImageTestCase("corrupt.jpg", "image/jpeg", IntSize(100, 100),
                        TEST_CASE_HAS_ERROR);
 }
 
 ImageTestCase CorruptBMPWithTruncatedHeader()
 {
--- a/image/test/gtest/Common.h
+++ b/image/test/gtest/Common.h
@@ -334,16 +334,33 @@ void AssertCorrectPipelineFinalState(Sur
  *              transparent.
  * @param aFuzz The amount of fuzz to use in pixel comparisons.
  */
 void CheckGeneratedImage(Decoder* aDecoder,
                          const gfx::IntRect& aRect,
                          uint8_t aFuzz = 0);
 
 /**
+ * Checks a generated surface for correctness. Reports any unexpected deviation
+ * from the expected image as GTest failures.
+ *
+ * @param aSurface The surface to check.
+ * @param aRect The region in the space of the output surface that the filter
+ *              pipeline will actually write to.
+ * @param aInnerColor Check that pixels inside of aRect are this color.
+ * @param aOuterColor Check that pixels outside of aRect are this color.
+ * @param aFuzz The amount of fuzz to use in pixel comparisons.
+ */
+void CheckGeneratedSurface(gfx::SourceSurface* aSurface,
+                           const gfx::IntRect& aRect,
+                           const BGRAColor& aInnerColor,
+                           const BGRAColor& aOuterColor,
+                           uint8_t aFuzz = 0);
+
+/**
  * Checks a generated paletted image for correctness. Reports any unexpected
  * deviation from the expected image as GTest failures.
  *
  * @param aDecoder The decoder which contains the image. The decoder's current
  *                 frame will be checked.
  * @param aRect The region in the space of the output surface that the filter
  *              pipeline will actually write to. It's expected that pixels in
  *              this region have a palette index of 255, while pixels outside
@@ -439,16 +456,20 @@ ImageTestCase GreenIconTestCase();
 ImageTestCase GreenWebPTestCase();
 
 ImageTestCase GreenWebPIccSrgbTestCase();
 
 ImageTestCase GreenFirstFrameAnimatedGIFTestCase();
 ImageTestCase GreenFirstFrameAnimatedPNGTestCase();
 ImageTestCase GreenFirstFrameAnimatedWebPTestCase();
 
+ImageTestCase BlendAnimatedGIFTestCase();
+ImageTestCase BlendAnimatedPNGTestCase();
+ImageTestCase BlendAnimatedWebPTestCase();
+
 ImageTestCase CorruptTestCase();
 ImageTestCase CorruptBMPWithTruncatedHeader();
 ImageTestCase CorruptICOWithBadBMPWidthTestCase();
 ImageTestCase CorruptICOWithBadBMPHeightTestCase();
 ImageTestCase CorruptICOWithBadBppTestCase();
 
 ImageTestCase TransparentPNGTestCase();
 ImageTestCase TransparentGIFTestCase();
new file mode 100644
--- /dev/null
+++ b/image/test/gtest/TestFrameAnimator.cpp
@@ -0,0 +1,149 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "gtest/gtest.h"
+
+#include "Common.h"
+#include "AnimationSurfaceProvider.h"
+#include "Decoder.h"
+#include "RasterImage.h"
+
+using namespace mozilla;
+using namespace mozilla::gfx;
+using namespace mozilla::image;
+
+static void
+CheckFrameAnimatorBlendResults(const ImageTestCase& aTestCase,
+                               RasterImage* aImage)
+{
+  // Allow the animation to actually begin.
+  aImage->IncrementAnimationConsumers();
+
+  // Initialize for the first frame so we can advance.
+  TimeStamp now = TimeStamp::Now();
+  aImage->RequestRefresh(now);
+
+  RefPtr<SourceSurface> surface =
+    aImage->GetFrame(imgIContainer::FRAME_CURRENT, imgIContainer::FLAG_NONE);
+  ASSERT_TRUE(surface != nullptr);
+
+  CheckGeneratedSurface(surface, IntRect(0, 0, 50, 50),
+                        BGRAColor::Transparent(),
+                        BGRAColor::Red());
+
+  // Advance to the next/final frame.
+  now = TimeStamp::Now() + TimeDuration::FromMilliseconds(500);
+  aImage->RequestRefresh(now);
+
+  surface =
+    aImage->GetFrame(imgIContainer::FRAME_CURRENT, imgIContainer::FLAG_NONE);
+  ASSERT_TRUE(surface != nullptr);
+  CheckGeneratedSurface(surface, IntRect(0, 0, 50, 50),
+                        BGRAColor::Green(),
+                        BGRAColor::Red());
+}
+
+template <typename Func>
+static void
+WithFrameAnimatorDecode(const ImageTestCase& aTestCase,
+                               bool aBlendFilter,
+                               Func aResultChecker)
+{
+  // Create an image.
+  RefPtr<Image> image =
+    ImageFactory::CreateAnonymousImage(nsDependentCString(aTestCase.mMimeType));
+  ASSERT_TRUE(!image->HasError());
+
+  NotNull<RefPtr<RasterImage>> rasterImage =
+    WrapNotNull(static_cast<RasterImage*>(image.get()));
+
+  nsCOMPtr<nsIInputStream> inputStream = LoadFile(aTestCase.mPath);
+  ASSERT_TRUE(inputStream != nullptr);
+
+  // Figure out how much data we have.
+  uint64_t length;
+  nsresult rv = inputStream->Available(&length);
+  ASSERT_TRUE(NS_SUCCEEDED(rv));
+
+  // Write the data into a SourceBuffer.
+  NotNull<RefPtr<SourceBuffer>> sourceBuffer = WrapNotNull(new SourceBuffer());
+  sourceBuffer->ExpectLength(length);
+  rv = sourceBuffer->AppendFromInputStream(inputStream, length);
+  ASSERT_TRUE(NS_SUCCEEDED(rv));
+  sourceBuffer->Complete(NS_OK);
+
+  // Create a metadata decoder first, because otherwise RasterImage will get
+  // unhappy about finding out the image is animated during a full decode.
+  DecoderType decoderType =
+    DecoderFactory::GetDecoderType(aTestCase.mMimeType);
+  RefPtr<IDecodingTask> task =
+    DecoderFactory::CreateMetadataDecoder(decoderType, rasterImage, sourceBuffer);
+  ASSERT_TRUE(task != nullptr);
+
+  // Run the metadata decoder synchronously.
+  task->Run();
+  task = nullptr;
+
+  // Create an AnimationSurfaceProvider which will manage the decoding process
+  // and make this decoder's output available in the surface cache.
+  DecoderFlags decoderFlags = DefaultDecoderFlags();
+  if (aBlendFilter) {
+    decoderFlags |= DecoderFlags::BLEND_ANIMATION;
+  }
+  SurfaceFlags surfaceFlags = DefaultSurfaceFlags();
+  rv = DecoderFactory::CreateAnimationDecoder(decoderType, rasterImage, sourceBuffer, aTestCase.mSize,
+                                              decoderFlags, surfaceFlags, 0, getter_AddRefs(task));
+  EXPECT_EQ(rv, NS_OK);
+  ASSERT_TRUE(task != nullptr);
+
+  // Run the full decoder synchronously.
+  task->Run();
+
+  // Call the lambda to verify the expected results.
+  aResultChecker(rasterImage.get());
+}
+
+static void
+CheckFrameAnimatorBlend(const ImageTestCase& aTestCase, bool aBlendFilter)
+{
+  WithFrameAnimatorDecode(aTestCase, aBlendFilter, [&](RasterImage* aImage) {
+    CheckFrameAnimatorBlendResults(aTestCase, aImage);
+  });
+}
+
+class ImageFrameAnimator : public ::testing::Test
+{
+protected:
+  AutoInitializeImageLib mInit;
+};
+
+TEST_F(ImageFrameAnimator, BlendGIFWithAnimator)
+{
+  CheckFrameAnimatorBlend(BlendAnimatedGIFTestCase(), /* aBlendFilter */ false);
+}
+
+TEST_F(ImageFrameAnimator, BlendGIFWithFilter)
+{
+  CheckFrameAnimatorBlend(BlendAnimatedGIFTestCase(), /* aBlendFilter */ true);
+}
+
+TEST_F(ImageFrameAnimator, BlendPNGWithAnimator)
+{
+  CheckFrameAnimatorBlend(BlendAnimatedPNGTestCase(), /* aBlendFilter */ false);
+}
+
+TEST_F(ImageFrameAnimator, BlendPNGWithFilter)
+{
+  CheckFrameAnimatorBlend(BlendAnimatedPNGTestCase(), /* aBlendFilter */ true);
+}
+
+TEST_F(ImageFrameAnimator, BlendWebPWithAnimator)
+{
+  CheckFrameAnimatorBlend(BlendAnimatedWebPTestCase(), /* aBlendFilter */ false);
+}
+
+TEST_F(ImageFrameAnimator, BlendWebPWithFilter)
+{
+  CheckFrameAnimatorBlend(BlendAnimatedWebPTestCase(), /* aBlendFilter */ true);
+}
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..2f7391454cf50e8524c35982212d63676bd03712
GIT binary patch
literal 412
zc${<hbhEHbOkqf2_`tyMp8*6Ef3h$dF)%VH{^$10O-xVqO-#>B&gM-|%*{=-HPACL
zH8C}1&;h9gX=7k|-P6DF^jwC;bGF>Fo_kN`A%9zt`?UU!Ws6Uiz3Se!iS7FR9{q1|
zPyZd{cB`{rIN^mc+eAy%{?6ie3yfFxYHhx@L07lu^rw9X)HZg-7%jh`QrdI2u=_!w
zWyija_doLe?Y;k?ol!y~!|_ByMaPN-Hx5@k+QG!BV(BTFQ0ckSvL*8Z=SNN{t67m&
zQd|kEPIdjdIqO%KB)9eHO72MS*{64O-+n2jW1SOyrkZEXt}}Pu{Na(+we^k7t?iw=
zcJJA{Z~yYj>Dl?k<<<4gTet7ry?6ik<n`_S<MZqL=da&?{{H*_{00jSG_r6@*>J1{
z2M&V~D3AySju8+x0E6`Z|Nl(JEx;gp%MS{o?#=i1{N`_a<T348=dxF)w!Py%{>kV1
ax8CQ!&VB#K&%@PzWYNbCA1!rO25SJU;<fq!
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..c4e739f068d54a03d4906ebbbe5f12ae5220e316
GIT binary patch
literal 339
zc%17D@N?(olHy`uVBq!ia0vp^DIm<j3?$ucQqzDGM`Ch_50GX8V#b!P#m|5=XMj(L
zE0F#V#$THE*MTHUg8YIR9G=|(>5@u==wg5w$p8aJAoeYeA5K8Fqo<2wNXEUlXE*W!
zEjn!Q<9}vehN@Y5zfGkyd!vJb00#>boI2FS%7iAir~Flktgo23{sSZ{8Nrr7Fp`ye
zegD#dZ1uDh#}JTrfYzP{S<Az;;Mo8Ecf5US8;VzIb}iBXqT;8Fp?`%t@2~me2UIFi
u;u=vBoS#-wo>-L1ke-=llvt3Lu2-C<mzP>H?Z5B|kR6_`elF{r5}E)NYgZuv
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..1b95e6f37710480c8eeab19d7ee23b98441caa96
GIT binary patch
literal 160
zc$^FJbaR`*z`zjh>J$(bU=hIuWD5atG8j4fdHS+3Kma3<=j&#}00R&eMnD!&pN}$-
z)=xelA;2&2S^l#;k168>1Cs^y;dj&?biMe`@*p&Jmj7Qq7qA%`a5D_S2xbO9$P7aV
S9tQRc|2zJ%D=qrZ$N&I7WFbia
--- a/image/test/gtest/moz.build
+++ b/image/test/gtest/moz.build
@@ -11,16 +11,17 @@ UNIFIED_SOURCES = [
     'TestADAM7InterpolatingFilter.cpp',
     'TestAnimationFrameBuffer.cpp',
     'TestBlendAnimationFilter.cpp',
     'TestContainers.cpp',
     'TestCopyOnWrite.cpp',
     'TestDecoders.cpp',
     'TestDecodeToSurface.cpp',
     'TestDeinterlacingFilter.cpp',
+    'TestFrameAnimator.cpp',
     'TestLoader.cpp',
     'TestMetadata.cpp',
     'TestRemoveFrameRectFilter.cpp',
     'TestSourceBuffer.cpp',
     'TestStreamingLexer.cpp',
     'TestSurfaceCache.cpp',
     'TestSurfaceSink.cpp',
 ]
@@ -33,16 +34,19 @@ if CONFIG['MOZ_ENABLE_SKIA']:
 
 SOURCES += [
     # Can't be unified because it manipulates the preprocessor environment.
     'TestDownscalingFilterNoSkia.cpp',
 ]
 
 TEST_HARNESS_FILES.gtest += [
     'animated-with-extra-image-sub-blocks.gif',
+    'blend.gif',
+    'blend.png',
+    'blend.webp',
     'corrupt-with-bad-bmp-height.ico',
     'corrupt-with-bad-bmp-width.ico',
     'corrupt-with-bad-ico-bpp.ico',
     'corrupt.jpg',
     'downscaled.bmp',
     'downscaled.gif',
     'downscaled.ico',
     'downscaled.icon',