Issue #2073 - m-c 1382683: Accelerate GIF decoding to SurfacePipe

1. Implement SurfacePipe::WritePixelBlocks for faster writing of pixels
2. Switch nsGIFDecoder2 to write pixels in blocks instead of individually

image/SurfacePipe.h

@@ -28,6 +28,7 @@
 #include "mozilla/Likely.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/Move.h"
+#include "mozilla/Tuple.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Unused.h"
 #include "mozilla/Variant.h"
@@ -175,6 +176,43 @@ public:
     return *result;
   }
 
+  /**
+   * Write pixels to the surface by calling a lambda which may write as many
+   * pixels as there is remaining to complete the row. It is not completely
+   * memory safe as it trusts the underlying decoder not to overrun the given
+   * buffer, however it is an acceptable tradeoff for performance.
+   *
+   * Writing continues until every pixel in the surface has been written to
+   * (i.e., IsSurfaceFinished() returns true) or the lambda returns a WriteState
+   * which WritePixelBlocks() will return to the caller.
+   *
+   * The template parameter PixelType must be uint8_t (for paletted surfaces) or
+   * uint32_t (for BGRA/BGRX surfaces) and must be in agreement with the pixel
+   * size passed to ConfigureFilter().
+   *
+   * XXX(seth): We'll remove all support for paletted surfaces in bug 1247520,
+   * which means we can remove the PixelType template parameter from this
+   * method.
+   *
+   * @param aFunc A lambda that functions as a generator, yielding at most the
+   *              maximum number of pixels requested. The lambda must accept a
+   *              pointer argument to the first pixel to write, a maximum
+   *              number of pixels to write as part of the block, and return a
+   *              NextPixel<PixelType> value.
+   *
+   * @return A WriteState value indicating the lambda generator's state.
+   *         WritePixelBlocks() itself will return WriteState::FINISHED if
+   *         writing has finished, regardless of the lambda's internal state.
+   */
+  template <typename PixelType, typename Func>
+  WriteState WritePixelBlocks(Func aFunc)
+  {
+    Maybe<WriteState> result;
+    while (!(result = DoWritePixelBlockToRow<PixelType>(Forward<Func>(aFunc)))) { }
+
+    return *result;
+  }
+
   /**
    * A variant of WritePixels() that writes a single row of pixels to the
    * surface one at a time by repeatedly calling a lambda that yields pixels.
@@ -449,6 +487,50 @@ protected:
 
 private:
 
+  /**
+   * An internal method used to implement WritePixelBlocks. This method writes
+   * up to the number of pixels necessary to complete the row and returns Some()
+   * if we either finished the entire surface or the lambda returned a
+   * WriteState indicating that we should return to the caller. If the row was
+   * successfully written without either of those things happening, it returns
+   * Nothing(), allowing WritePixelBlocks() to iterate to fill as many rows as
+   * possible.
+   */
+  template <typename PixelType, typename Func>
+  Maybe<WriteState> DoWritePixelBlockToRow(Func aFunc)
+  {
+    MOZ_ASSERT(mPixelSize == 1 || mPixelSize == 4);
+    MOZ_ASSERT_IF(mPixelSize == 1, sizeof(PixelType) == sizeof(uint8_t));
+    MOZ_ASSERT_IF(mPixelSize == 4, sizeof(PixelType) == sizeof(uint32_t));
+
+    if (IsSurfaceFinished()) {
+      return Some(WriteState::FINISHED);  // We're already done.
+    }
+
+    PixelType* rowPtr = reinterpret_cast<PixelType*>(mRowPointer);
+    int32_t remainder = mInputSize.width - mCol;
+    int32_t written;
+    Maybe<WriteState> result;
+    Tie(written, result) = aFunc(&rowPtr[mCol], remainder);
+    if (written == remainder) {
+      MOZ_ASSERT(result.isNothing());
+      mCol = mInputSize.width;
+      AdvanceRow();  // We've finished the row.
+      return IsSurfaceFinished() ? Some(WriteState::FINISHED)
+                                 : Nothing();
+    }
+
+    MOZ_ASSERT(written >= 0 && written < remainder);
+    MOZ_ASSERT(result.isSome());
+
+    mCol += written;
+    if (*result == WriteState::FINISHED) {
+      ZeroOutRestOfSurface<PixelType>();
+    }
+
+    return result;
+  }
+
   /**
    * An internal method used to implement both WritePixels() and
    * WritePixelsToRow(). Those methods differ only in their behavior after a row
@@ -607,6 +689,20 @@ public:
     return mHead->WritePixels<PixelType>(Forward<Func>(aFunc));
   }
 
+  /**
+   * A variant of WritePixels() that writes up to a single row of pixels to the
+   * surface in blocks by repeatedly calling a lambda that yields up to the
+   * requested number of pixels.
+   *
+   * @see SurfaceFilter::WritePixelBlocks() for the canonical documentation.
+   */
+  template <typename PixelType, typename Func>
+  WriteState WritePixelBlocks(Func aFunc)
+  {
+    MOZ_ASSERT(mHead, "Use before configured!");
+    return mHead->WritePixelBlocks<PixelType>(Forward<Func>(aFunc));
+  }
+
   /**
    * A variant of WritePixels() that writes a single row of pixels to the
    * surface one at a time by repeatedly calling a lambda that yields pixels.

image/decoders/nsGIFDecoder2.cpp

@@ -299,10 +299,12 @@ nsGIFDecoder2::ColormapIndexToPixel<uint8_t>(uint8_t aIndex)
 }
 
 template <typename PixelSize>
-NextPixel<PixelSize>
+Tuple<int32_t, Maybe<WriteState>>
-nsGIFDecoder2::YieldPixel(const uint8_t* aData,
+nsGIFDecoder2::YieldPixels(const uint8_t* aData,
-                          size_t aLength,
+                           size_t aLength,
-                          size_t* aBytesReadOut)
+                           size_t* aBytesReadOut,
+                           PixelSize* aPixelBlock,
+                           int32_t aBlockSize)
 {
   MOZ_ASSERT(aData);
   MOZ_ASSERT(aBytesReadOut);
@@ -311,108 +313,119 @@ nsGIFDecoder2::YieldPixel(const uint8_t* aData,
   // Advance to the next byte we should read.
   const uint8_t* data = aData + *aBytesReadOut;
 
-  // If we don't have any decoded data to yield, try to read some input and
+  int32_t written = 0;
-  // produce some.
+  while (aBlockSize > written) {
-  if (mGIFStruct.stackp == mGIFStruct.stack) {
+    // If we don't have any decoded data to yield, try to read some input and
-    while (mGIFStruct.bits < mGIFStruct.codesize && *aBytesReadOut < aLength) {
+    // produce some.
-      // Feed the next byte into the decoder's 32-bit input buffer.
+    if (mGIFStruct.stackp == mGIFStruct.stack) {
-      mGIFStruct.datum += int32_t(*data) << mGIFStruct.bits;
+      while (mGIFStruct.bits < mGIFStruct.codesize && *aBytesReadOut < aLength) {
-      mGIFStruct.bits += 8;
+        // Feed the next byte into the decoder's 32-bit input buffer.
-      data += 1;
+        mGIFStruct.datum += int32_t(*data) << mGIFStruct.bits;
-      *aBytesReadOut += 1;
+        mGIFStruct.bits += 8;
-    }
+        data += 1;
-
+        *aBytesReadOut += 1;
-    if (mGIFStruct.bits < mGIFStruct.codesize) {
-      return AsVariant(WriteState::NEED_MORE_DATA);
-    }
-
-    // Get the leading variable-length symbol from the data stream.
-    int code = mGIFStruct.datum & mGIFStruct.codemask;
-    mGIFStruct.datum >>= mGIFStruct.codesize;
-    mGIFStruct.bits -= mGIFStruct.codesize;
-
-    const int clearCode = ClearCode();
-
-    // Reset the dictionary to its original state, if requested
-    if (code == clearCode) {
-      mGIFStruct.codesize = mGIFStruct.datasize + 1;
-      mGIFStruct.codemask = (1 << mGIFStruct.codesize) - 1;
-      mGIFStruct.avail = clearCode + 2;
-      mGIFStruct.oldcode = -1;
-      return AsVariant(WriteState::NEED_MORE_DATA);
-    }
-
-    // Check for explicit end-of-stream code. It should only appear after all
-    // image data, but if that was the case we wouldn't be in this function, so
-    // this is always an error condition.
-    if (code == (clearCode + 1)) {
-      return AsVariant(WriteState::FAILURE);
-    }
-
-    if (mGIFStruct.oldcode == -1) {
-      if (code >= MAX_BITS) {
-        return AsVariant(WriteState::FAILURE);  // The code's too big; something's wrong.
       }
 
-      mGIFStruct.firstchar = mGIFStruct.oldcode = code;
+      if (mGIFStruct.bits < mGIFStruct.codesize) {
-
+        return MakeTuple(written, Some(WriteState::NEED_MORE_DATA));
-      // Yield a pixel at the appropriate index in the colormap.
-      mGIFStruct.pixels_remaining--;
-      return AsVariant(ColormapIndexToPixel<PixelSize>(mGIFStruct.suffix[code]));
-    }
-
-    int incode = code;
-    if (code >= mGIFStruct.avail) {
-      *mGIFStruct.stackp++ = mGIFStruct.firstchar;
-      code = mGIFStruct.oldcode;
-
-      if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) {
-        return AsVariant(WriteState::FAILURE);  // Stack overflow; something's wrong.
-      }
-    }
-
-    while (code >= clearCode) {
-      if ((code >= MAX_BITS) || (code == mGIFStruct.prefix[code])) {
-        return AsVariant(WriteState::FAILURE);
       }
 
-      *mGIFStruct.stackp++ = mGIFStruct.suffix[code];
+      // Get the leading variable-length symbol from the data stream.
-      code = mGIFStruct.prefix[code];
+      int code = mGIFStruct.datum & mGIFStruct.codemask;
+      mGIFStruct.datum >>= mGIFStruct.codesize;
+      mGIFStruct.bits -= mGIFStruct.codesize;
 
-      if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) {
+      const int clearCode = ClearCode();
-        return AsVariant(WriteState::FAILURE);  // Stack overflow; something's wrong.
+
+      // Reset the dictionary to its original state, if requested
+      if (code == clearCode) {
+        mGIFStruct.codesize = mGIFStruct.datasize + 1;
+        mGIFStruct.codemask = (1 << mGIFStruct.codesize) - 1;
+        mGIFStruct.avail = clearCode + 2;
+        mGIFStruct.oldcode = -1;
+        return MakeTuple(written, Some(WriteState::NEED_MORE_DATA));
       }
+
+      // Check for explicit end-of-stream code. It should only appear after all
+      // image data, but if that was the case we wouldn't be in this function, so
+      // this is always an error condition.
+      if (code == (clearCode + 1)) {
+        return MakeTuple(written, Some(WriteState::FAILURE));
+      }
+
+      if (mGIFStruct.oldcode == -1) {
+        if (code >= MAX_BITS) {
+          // The code's too big; something's wrong.
+          return MakeTuple(written, Some(WriteState::FAILURE));
+        }
+
+        mGIFStruct.firstchar = mGIFStruct.oldcode = code;
+
+        // Yield a pixel at the appropriate index in the colormap.
+        mGIFStruct.pixels_remaining--;
+        aPixelBlock[written++] =
+          ColormapIndexToPixel<PixelSize>(mGIFStruct.suffix[code]);
+        continue;
+      }
+
+      int incode = code;
+      if (code >= mGIFStruct.avail) {
+        *mGIFStruct.stackp++ = mGIFStruct.firstchar;
+        code = mGIFStruct.oldcode;
+
+        if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) {
+          // Stack overflow; something's wrong.
+          return MakeTuple(written, Some(WriteState::FAILURE));
+        }
+      }
+
+      while (code >= clearCode) {
+        if ((code >= MAX_BITS) || (code == mGIFStruct.prefix[code])) {
+          return MakeTuple(written, Some(WriteState::FAILURE));
+        }
+
+        *mGIFStruct.stackp++ = mGIFStruct.suffix[code];
+        code = mGIFStruct.prefix[code];
+
+        if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) {
+          // Stack overflow; something's wrong.
+          return MakeTuple(written, Some(WriteState::FAILURE));
+        }
+      }
+
+      *mGIFStruct.stackp++ = mGIFStruct.firstchar = mGIFStruct.suffix[code];
+
+      // Define a new codeword in the dictionary.
+      if (mGIFStruct.avail < 4096) {
+        mGIFStruct.prefix[mGIFStruct.avail] = mGIFStruct.oldcode;
+        mGIFStruct.suffix[mGIFStruct.avail] = mGIFStruct.firstchar;
+        mGIFStruct.avail++;
+
+        // If we've used up all the codewords of a given length increase the
+        // length of codewords by one bit, but don't exceed the specified maximum
+        // codeword size of 12 bits.
+        if (((mGIFStruct.avail & mGIFStruct.codemask) == 0) &&
+            (mGIFStruct.avail < 4096)) {
+          mGIFStruct.codesize++;
+          mGIFStruct.codemask += mGIFStruct.avail;
+        }
+      }
+
+      mGIFStruct.oldcode = incode;
     }
 
-    *mGIFStruct.stackp++ = mGIFStruct.firstchar = mGIFStruct.suffix[code];
+    if (MOZ_UNLIKELY(mGIFStruct.stackp <= mGIFStruct.stack)) {
-
+      MOZ_ASSERT_UNREACHABLE("No decoded data but we didn't return early?");
-    // Define a new codeword in the dictionary.
+      return MakeTuple(written, Some(WriteState::FAILURE));
-    if (mGIFStruct.avail < 4096) {
-      mGIFStruct.prefix[mGIFStruct.avail] = mGIFStruct.oldcode;
-      mGIFStruct.suffix[mGIFStruct.avail] = mGIFStruct.firstchar;
-      mGIFStruct.avail++;
-
-      // If we've used up all the codewords of a given length increase the
-      // length of codewords by one bit, but don't exceed the specified maximum
-      // codeword size of 12 bits.
-      if (((mGIFStruct.avail & mGIFStruct.codemask) == 0) &&
-          (mGIFStruct.avail < 4096)) {
-        mGIFStruct.codesize++;
-        mGIFStruct.codemask += mGIFStruct.avail;
-      }
     }
 
-    mGIFStruct.oldcode = incode;
+    // Yield a pixel at the appropriate index in the colormap.
+    mGIFStruct.pixels_remaining--;
+    aPixelBlock[written++]
+      = ColormapIndexToPixel<PixelSize>(*--mGIFStruct.stackp);
   }
 
-  if (MOZ_UNLIKELY(mGIFStruct.stackp <= mGIFStruct.stack)) {
+  return MakeTuple(written, Maybe<WriteState>());
-    MOZ_ASSERT_UNREACHABLE("No decoded data but we didn't return early?");
-    return AsVariant(WriteState::FAILURE);
-  }
-
-  // Yield a pixel at the appropriate index in the colormap.
-  mGIFStruct.pixels_remaining--;
-  return AsVariant(ColormapIndexToPixel<PixelSize>(*--mGIFStruct.stackp));
 }
 
 /// Expand the colormap from RGB to Packed ARGB as needed by Cairo.
@@ -1030,8 +1043,12 @@ nsGIFDecoder2::ReadLZWData(const char* aData, size_t aLength)
     size_t bytesRead = 0;
 
     auto result = mGIFStruct.images_decoded == 0
-      ? mPipe.WritePixels<uint32_t>([&]{ return YieldPixel<uint32_t>(data, length, &bytesRead); })
+      ? mPipe.WritePixelBlocks<uint32_t>([&](uint32_t* aPixelBlock, int32_t aBlockSize) {
-      : mPipe.WritePixels<uint8_t>([&]{ return YieldPixel<uint8_t>(data, length, &bytesRead); });
+          return YieldPixels<uint32_t>(data, length, &bytesRead, aPixelBlock, aBlockSize);
+        })
+      : mPipe.WritePixelBlocks<uint8_t>([&](uint8_t* aPixelBlock, int32_t aBlockSize) {
+          return YieldPixels<uint8_t>(data, length, &bytesRead, aPixelBlock, aBlockSize);
+        });
 
     if (MOZ_UNLIKELY(bytesRead > length)) {
       MOZ_ASSERT_UNREACHABLE("Overread?");

image/decoders/nsGIFDecoder2.h

@@ -61,8 +61,12 @@ private:
   ColormapIndexToPixel(uint8_t aIndex);
 
   /// A generator function that performs LZW decompression and yields pixels.
-  template <typename PixelSize> NextPixel<PixelSize>
+  template <typename PixelSize> Tuple<int32_t, Maybe<WriteState>>
-  YieldPixel(const uint8_t* aData, size_t aLength, size_t* aBytesReadOut);
+  YieldPixels(const uint8_t* aData,
+              size_t aLength,
+              size_t* aBytesReadOut,
+              PixelSize* aPixelBlock,
+              int32_t aBlockSize);
 
   /// Checks if we have transparency, either because the header indicates that
   /// there's alpha, or because the frame rect doesn't cover the entire image.