Bug 1279617 - When SurfacePipe::WritePixels() finishes early, zero out the rest of the surface. r=njn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 17 Jun 2016 15:05:00 -0700
changeset 302047 35530c11c1631b62e2e02c59e071f49d877b1184
parent 302046 78f55d79c3e04e9781ee58228a21c19d9a0f59f9
child 302048 ada00a4c36c5ba384e2fdc2c4bd94f20fe7c744c
push id30349
push usercbook@mozilla.com
push dateMon, 20 Jun 2016 11:51:16 +0000
treeherdermozilla-central@3c5025f98e56 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1279617
milestone50.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 1279617 - When SurfacePipe::WritePixels() finishes early, zero out the rest of the surface. r=njn
image/SurfacePipe.h
image/test/gtest/TestSurfaceSink.cpp
--- a/image/SurfacePipe.h
+++ b/image/SurfacePipe.h
@@ -180,20 +180,17 @@ public:
           continue;
         }
 
         switch (result.template as<WriteState>()) {
           case WriteState::NEED_MORE_DATA:
             return WriteState::NEED_MORE_DATA;
 
           case WriteState::FINISHED:
-            // Make sure that IsSurfaceFinished() returns true so the caller
-            // can't write anything else to the pipeline.
-            mRowPointer = nullptr;
-            mCol = 0;
+            ZeroOutRestOfSurface<PixelType>();
             return WriteState::FINISHED;
 
           case WriteState::FAILURE:
             // Note that we don't need to record this anywhere, because this
             // indicates an error in aFunc, and there's nothing wrong with our
             // machinery. The caller can recover as needed and continue writing to
             // the row.
             return WriteState::FAILURE;
@@ -439,16 +436,23 @@ protected:
   {
     mInputSize = aInputSize;
     mPixelSize = aPixelSize;
 
     ResetToFirstRow();
   }
 
 private:
+
+  template <typename PixelType>
+  void ZeroOutRestOfSurface()
+  {
+    WritePixels<PixelType>([]{ return AsVariant(PixelType(0)); });
+  }
+
   gfx::IntSize mInputSize;  /// The size of the input this filter expects.
   uint8_t* mRowPointer;     /// Pointer to the current row or null if finished.
   int32_t mCol;             /// The current column we're writing to. (0-indexed)
   uint8_t  mPixelSize;      /// How large each pixel in the surface is, in bytes.
 };
 
 class NullSurfaceSink;
 
--- a/image/test/gtest/TestSurfaceSink.cpp
+++ b/image/test/gtest/TestSurfaceSink.cpp
@@ -208,17 +208,20 @@ TEST(ImageSurfaceSink, SurfaceSinkWriteP
     // Write nothing into the surface; just finish immediately.
     uint32_t count = 0;
     auto result = aSink->WritePixels<uint32_t>([&]() {
       count++;
       return AsVariant(WriteState::FINISHED);
     });
     EXPECT_EQ(WriteState::FINISHED, result);
     EXPECT_EQ(1u, count);
-    EXPECT_TRUE(aSink->IsSurfaceFinished());
+
+    AssertCorrectPipelineFinalState(aSink,
+                                    IntRect(0, 0, 100, 100),
+                                    IntRect(0, 0, 100, 100));
 
     // Attempt to write more and make sure that nothing gets written.
     count = 0;
     result = aSink->WritePixels<uint32_t>([&]() {
       count++;
       return AsVariant(BGRAColor::Red().AsPixel());
     });
     EXPECT_EQ(WriteState::FINISHED, result);
@@ -227,16 +230,91 @@ TEST(ImageSurfaceSink, SurfaceSinkWriteP
 
     // Check that the generated image is correct.
     RawAccessFrameRef currentFrame = aDecoder->GetCurrentFrameRef();
     RefPtr<SourceSurface> surface = currentFrame->GetSurface();
     EXPECT_TRUE(IsSolidColor(surface, BGRAColor::Transparent()));
   });
 }
 
+TEST(ImageSurfaceSink, SurfaceSinkWritePixelsEarlyExit)
+{
+  auto checkEarlyExit =
+    [](Decoder* aDecoder, SurfaceSink* aSink, WriteState aState) {
+    // Write half a row of green pixels and then exit early with |aState|. If
+    // the lambda keeps getting called, we'll write red pixels, which will cause
+    // the test to fail.
+    uint32_t count = 0;
+    auto result = aSink->WritePixels<uint32_t>([&]() -> NextPixel<uint32_t> {
+      if (count == 50) {
+        return AsVariant(aState);
+      }
+      return count++ < 50 ? AsVariant(BGRAColor::Green().AsPixel())
+                          : AsVariant(BGRAColor::Red().AsPixel());
+    });
+
+    EXPECT_EQ(aState, result);
+    EXPECT_EQ(50u, count);
+    CheckGeneratedImage(aDecoder, IntRect(0, 0, 50, 1));
+
+    if (aState != WriteState::FINISHED) {
+      // We should still be able to write more at this point.
+      EXPECT_FALSE(aSink->IsSurfaceFinished());
+
+      // Verify that we can resume writing. We'll finish up the same row.
+      count = 0;
+      result = aSink->WritePixels<uint32_t>([&]() -> NextPixel<uint32_t> {
+        if (count == 50) {
+          return AsVariant(WriteState::NEED_MORE_DATA);
+        }
+        ++count;
+        return AsVariant(BGRAColor::Green().AsPixel());
+      });
+
+      EXPECT_EQ(WriteState::NEED_MORE_DATA, result);
+      EXPECT_EQ(50u, count);
+      EXPECT_FALSE(aSink->IsSurfaceFinished());
+      CheckGeneratedImage(aDecoder, IntRect(0, 0, 100, 1));
+
+      return;
+    }
+
+    // We should've finished the surface at this point.
+    AssertCorrectPipelineFinalState(aSink,
+                                    IntRect(0, 0, 100, 100),
+                                    IntRect(0, 0, 100, 100));
+
+    // Attempt to write more and make sure that nothing gets written.
+    count = 0;
+    result = aSink->WritePixels<uint32_t>([&]{
+      count++;
+      return AsVariant(BGRAColor::Red().AsPixel());
+    });
+
+    EXPECT_EQ(WriteState::FINISHED, result);
+    EXPECT_EQ(0u, count);
+    EXPECT_TRUE(aSink->IsSurfaceFinished());
+
+    // Check that the generated image is still correct.
+    CheckGeneratedImage(aDecoder, IntRect(0, 0, 50, 1));
+  };
+
+  WithSurfaceSink<Orient::NORMAL>([&](Decoder* aDecoder, SurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::NEED_MORE_DATA);
+  });
+
+  WithSurfaceSink<Orient::NORMAL>([&](Decoder* aDecoder, SurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::FAILURE);
+  });
+
+  WithSurfaceSink<Orient::NORMAL>([&](Decoder* aDecoder, SurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::FINISHED);
+  });
+}
+
 TEST(ImageSurfaceSink, SurfaceSinkWriteBuffer)
 {
   WithSurfaceSink<Orient::NORMAL>([](Decoder* aDecoder, SurfaceSink* aSink) {
     // Create a green buffer the same size as one row of the surface (which is 100x100),
     // containing 60 pixels of green in the middle and 20 transparent pixels on
     // either side.
     uint32_t buffer[100];
     for (int i = 0; i < 100; ++i) {
@@ -804,16 +882,125 @@ TEST(ImageSurfaceSink, PalettedSurfaceSi
     CheckPalettedWritePixels(aDecoder, aSink,
                              /* aOutputRect = */ Some(IntRect(0, 0, 50, 50)),
                              /* aInputRect = */ Some(IntRect(0, 0, 50, 50)),
                              /* aInputWriteRect = */ Some(IntRect(75, 75, 50, 50)),
                              /* aOutputWriteRect = */ Some(IntRect(75, 75, 50, 50)));
   });
 }
 
+TEST(ImageSurfaceSink, PalettedSurfaceSinkWritePixelsFinish)
+{
+  WithPalettedSurfaceSink(IntRect(0, 0, 100, 100),
+                          [](Decoder* aDecoder, PalettedSurfaceSink* aSink) {
+    // Write nothing into the surface; just finish immediately.
+    uint32_t count = 0;
+    auto result = aSink->WritePixels<uint8_t>([&]{
+      count++;
+      return AsVariant(WriteState::FINISHED);
+    });
+    EXPECT_EQ(WriteState::FINISHED, result);
+    EXPECT_EQ(1u, count);
+
+    AssertCorrectPipelineFinalState(aSink,
+                                    IntRect(0, 0, 100, 100),
+                                    IntRect(0, 0, 100, 100));
+
+    // Attempt to write more and make sure that nothing gets written.
+    count = 0;
+    result = aSink->WritePixels<uint8_t>([&]() {
+      count++;
+      return AsVariant(uint8_t(128));
+    });
+    EXPECT_EQ(WriteState::FINISHED, result);
+    EXPECT_EQ(0u, count);
+    EXPECT_TRUE(aSink->IsSurfaceFinished());
+
+    // Check that the generated image is correct.
+    EXPECT_TRUE(IsSolidPalettedColor(aDecoder, 0));
+  });
+}
+
+TEST(ImageSurfaceSink, PalettedSurfaceSinkWritePixelsEarlyExit)
+{
+  auto checkEarlyExit =
+    [](Decoder* aDecoder, PalettedSurfaceSink* aSink, WriteState aState) {
+    // Write half a row of green pixels and then exit early with |aState|. If
+    // the lambda keeps getting called, we'll write red pixels, which will cause
+    // the test to fail.
+    uint32_t count = 0;
+    auto result = aSink->WritePixels<uint8_t>([&]() -> NextPixel<uint8_t> {
+      if (count == 50) {
+        return AsVariant(aState);
+      }
+      return count++ < 50 ? AsVariant(uint8_t(255)) : AsVariant(uint8_t(128));
+    });
+
+    EXPECT_EQ(aState, result);
+    EXPECT_EQ(50u, count);
+    CheckGeneratedPalettedImage(aDecoder, IntRect(0, 0, 50, 1));
+
+    if (aState != WriteState::FINISHED) {
+      // We should still be able to write more at this point.
+      EXPECT_FALSE(aSink->IsSurfaceFinished());
+
+      // Verify that we can resume writing. We'll finish up the same row.
+      count = 0;
+      result = aSink->WritePixels<uint8_t>([&]() -> NextPixel<uint8_t> {
+        if (count == 50) {
+          return AsVariant(WriteState::NEED_MORE_DATA);
+        }
+        ++count;
+        return AsVariant(uint8_t(255));
+      });
+
+      EXPECT_EQ(WriteState::NEED_MORE_DATA, result);
+      EXPECT_EQ(50u, count);
+      EXPECT_FALSE(aSink->IsSurfaceFinished());
+      CheckGeneratedPalettedImage(aDecoder, IntRect(0, 0, 100, 1));
+
+      return;
+    }
+
+    // We should've finished the surface at this point.
+    AssertCorrectPipelineFinalState(aSink,
+                                    IntRect(0, 0, 100, 100),
+                                    IntRect(0, 0, 100, 100));
+
+    // Attempt to write more and make sure that nothing gets written.
+    count = 0;
+    result = aSink->WritePixels<uint8_t>([&]{
+      count++;
+      return AsVariant(uint8_t(128));
+    });
+
+    EXPECT_EQ(WriteState::FINISHED, result);
+    EXPECT_EQ(0u, count);
+    EXPECT_TRUE(aSink->IsSurfaceFinished());
+
+    // Check that the generated image is still correct.
+    CheckGeneratedPalettedImage(aDecoder, IntRect(0, 0, 50, 1));
+  };
+
+  WithPalettedSurfaceSink(IntRect(0, 0, 100, 100),
+                          [&](Decoder* aDecoder, PalettedSurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::NEED_MORE_DATA);
+  });
+
+  WithPalettedSurfaceSink(IntRect(0, 0, 100, 100),
+                          [&](Decoder* aDecoder, PalettedSurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::FAILURE);
+  });
+
+  WithPalettedSurfaceSink(IntRect(0, 0, 100, 100),
+                          [&](Decoder* aDecoder, PalettedSurfaceSink* aSink) {
+    checkEarlyExit(aDecoder, aSink, WriteState::FINISHED);
+  });
+}
+
 TEST(ImageSurfaceSink, PalettedSurfaceSinkWriteBuffer)
 {
   WithPalettedSurfaceSink(IntRect(0, 0, 100, 100),
                           [](Decoder* aDecoder, PalettedSurfaceSink* aSink) {
     // Create a buffer the same size as one row of the surface (which is 100x100),
     // containing 60 pixels of 255 in the middle and 20 transparent pixels of 0 on
     // either side.
     uint8_t buffer[100];