Bug 1551088 - Part 5. Expose SurfaceFilter input row directly to avoid copy if possible. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 24 Sep 2019 20:42:45 +0000
changeset 494821 f77c43bcc75bf8d74b9d5423e318ff106fc93fd5
parent 494820 9e954d6765de72a2c4000b657b2e59afd585a540
child 494822 5d72c8de4dafd142392b65e11cf22faba5064ccb
push id114131
push userdluca@mozilla.com
push dateThu, 26 Sep 2019 09:47:34 +0000
treeherdermozilla-inbound@1dc1a755079a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1551088
milestone71.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 1551088 - Part 5. Expose SurfaceFilter input row directly to avoid copy if possible. r=tnikkel Some filters can do the copy of the given data into the working buffer as part of the filter operation. For those that cannot, we will just copy the data first, and then advance the row. Differential Revision: https://phabricator.services.mozilla.com/D46448
image/DownscalingFilter.h
image/SurfaceFilters.h
image/SurfacePipe.cpp
image/SurfacePipe.h
--- a/image/DownscalingFilter.h
+++ b/image/DownscalingFilter.h
@@ -73,16 +73,20 @@ class DownscalingFilter final : public S
     return NS_ERROR_FAILURE;
   }
 
  protected:
   uint8_t* DoResetToFirstRow() override {
     MOZ_CRASH();
     return nullptr;
   }
+  uint8_t* DoAdvanceRowFromBuffer(const uint8_t* aInputRow) override {
+    MOZ_CRASH();
+    return nullptr;
+  }
   uint8_t* DoAdvanceRow() override {
     MOZ_CRASH();
     return nullptr;
   }
 };
 
 #else
 
@@ -201,17 +205,17 @@ class DownscalingFilter final : public S
 
     mInputRow = 0;
     mOutputRow = 0;
     mRowsInWindow = 0;
 
     return GetRowPointer();
   }
 
-  uint8_t* DoAdvanceRow() override {
+  uint8_t* DoAdvanceRowFromBuffer(const uint8_t* aInputRow) override {
     if (mInputRow >= mInputSize.height) {
       NS_WARNING("Advancing DownscalingFilter past the end of the input");
       return nullptr;
     }
 
     if (mOutputRow >= mNext.InputSize().height) {
       NS_WARNING("Advancing DownscalingFilter past the end of the output");
       return nullptr;
@@ -221,17 +225,17 @@ class DownscalingFilter final : public S
     int32_t filterLength = 0;
     mYFilter.GetFilterOffsetAndLength(mOutputRow, &filterOffset, &filterLength);
 
     int32_t inputRowToRead = filterOffset + mRowsInWindow;
     MOZ_ASSERT(mInputRow <= inputRowToRead, "Reading past end of input");
     if (mInputRow == inputRowToRead) {
       MOZ_RELEASE_ASSERT(mRowsInWindow < mWindowCapacity,
                          "Need more rows than capacity!");
-      mXFilter.ConvolveHorizontally(mRowBuffer.get(), mWindow[mRowsInWindow++],
+      mXFilter.ConvolveHorizontally(aInputRow, mWindow[mRowsInWindow++],
                                     mHasAlpha);
     }
 
     MOZ_ASSERT(mOutputRow < mNext.InputSize().height,
                "Writing past end of output");
 
     while (mRowsInWindow >= filterLength) {
       DownscaleInputRow();
@@ -244,16 +248,20 @@ class DownscalingFilter final : public S
                                         &filterLength);
     }
 
     mInputRow++;
 
     return mInputRow < mInputSize.height ? GetRowPointer() : nullptr;
   }
 
+  uint8_t* DoAdvanceRow() override {
+    return DoAdvanceRowFromBuffer(mRowBuffer.get());
+  }
+
  private:
   uint8_t* GetRowPointer() const { return mRowBuffer.get(); }
 
   static size_t PaddedWidthInBytes(size_t aLogicalWidth) {
     // Convert from width in BGRA/BGRX pixels to width in bytes, padding
     // to handle overreads by the SIMD code inside Skia.
     return gfx::ConvolutionFilter::PadBytesForSIMD(aLogicalWidth *
                                                    sizeof(uint32_t));
--- a/image/SurfaceFilters.h
+++ b/image/SurfaceFilters.h
@@ -75,20 +75,24 @@ class ColorManagementFilter final : publ
 
   Maybe<SurfaceInvalidRect> TakeInvalidRect() override {
     return mNext.TakeInvalidRect();
   }
 
  protected:
   uint8_t* DoResetToFirstRow() override { return mNext.ResetToFirstRow(); }
 
+  uint8_t* DoAdvanceRowFromBuffer(const uint8_t* aInputRow) override {
+    qcms_transform_data(mTransform, aInputRow, mNext.CurrentRowPointer(),
+                        mNext.InputSize().width);
+    return mNext.AdvanceRow();
+  }
+
   uint8_t* DoAdvanceRow() override {
-    uint8_t* rowPtr = mNext.CurrentRowPointer();
-    qcms_transform_data(mTransform, rowPtr, rowPtr, mNext.InputSize().width);
-    return mNext.AdvanceRow();
+    return DoAdvanceRowFromBuffer(mNext.CurrentRowPointer());
   }
 
   Next mNext;  /// The next SurfaceFilter in the chain.
 
   qcms_transform* mTransform;
 };
 
 //////////////////////////////////////////////////////////////////////////////
@@ -177,16 +181,21 @@ class DeinterlacingFilter final : public
   uint8_t* DoResetToFirstRow() override {
     mNext.ResetToFirstRow();
     mPass = 0;
     mInputRow = 0;
     mOutputRow = InterlaceOffset(mPass);
     return GetRowPointer(mOutputRow);
   }
 
+  uint8_t* DoAdvanceRowFromBuffer(const uint8_t* aInputRow) override {
+    CopyInputRow(aInputRow);
+    return DoAdvanceRow();
+  }
+
   uint8_t* DoAdvanceRow() override {
     if (mPass >= 4) {
       return nullptr;  // We already finished all passes.
     }
     if (mInputRow >= InputSize().height) {
       return nullptr;  // We already got all the input rows we expect.
     }
 
@@ -633,16 +642,21 @@ class BlendAnimationFilter final : publi
     // to the end of the next pipeline stage's buffer, outputting rows that are
     // copied from the base frame and/or cleared.
     WriteBaseFrameRowsUntilComplete();
 
     mRow = mFrameRect.YMost();
     return nullptr;  // We're done.
   }
 
+  uint8_t* DoAdvanceRowFromBuffer(const uint8_t* aInputRow) override {
+    CopyInputRow(aInputRow);
+    return DoAdvanceRow();
+  }
+
   uint8_t* DoAdvanceRow() override {
     uint8_t* rowPtr = nullptr;
 
     const int32_t currentRow = mRow;
     mRow++;
 
     // The unclamped frame rect has a negative offset which means -y rows from
     // the decoder need to be discarded before we advance properly.
@@ -907,16 +921,21 @@ class RemoveFrameRectFilter final : publ
     // to the end of the next pipeline stage's buffer, outputting blank rows.
     while (mNext.WriteEmptyRow() == WriteState::NEED_MORE_DATA) {
     }
 
     mRow = mFrameRect.YMost();
     return nullptr;  // We're done.
   }
 
+  uint8_t* DoAdvanceRowFromBuffer(const uint8_t* aInputRow) override {
+    CopyInputRow(aInputRow);
+    return DoAdvanceRow();
+  }
+
   uint8_t* DoAdvanceRow() override {
     uint8_t* rowPtr = nullptr;
 
     const int32_t currentRow = mRow;
     mRow++;
 
     if (currentRow < mFrameRect.Y()) {
       // This row is outside of the frame rect, so just drop it on the floor.
@@ -1092,16 +1111,21 @@ class ADAM7InterpolatingFilter final : p
       // Short circuit this filter on the final pass, since all pixels have
       // their final values at that point.
       return rowPtr;
     }
 
     return mCurrentRow.get();
   }
 
+  uint8_t* DoAdvanceRowFromBuffer(const uint8_t* aInputRow) override {
+    CopyInputRow(aInputRow);
+    return DoAdvanceRow();
+  }
+
   uint8_t* DoAdvanceRow() override {
     MOZ_ASSERT(0 < mPass && mPass <= 7, "Invalid pass");
 
     int32_t currentRow = mRow;
     ++mRow;
 
     if (mPass == 7) {
       // On the final pass we short circuit this filter totally.
--- a/image/SurfacePipe.cpp
+++ b/image/SurfacePipe.cpp
@@ -31,16 +31,21 @@ Maybe<SurfaceInvalidRect> AbstractSurfac
   return Some(invalidRect);
 }
 
 uint8_t* AbstractSurfaceSink::DoResetToFirstRow() {
   mRow = 0;
   return GetRowPointer();
 }
 
+uint8_t* AbstractSurfaceSink::DoAdvanceRowFromBuffer(const uint8_t* aInputRow) {
+  CopyInputRow(aInputRow);
+  return DoAdvanceRow();
+}
+
 uint8_t* AbstractSurfaceSink::DoAdvanceRow() {
   if (mRow >= uint32_t(InputSize().height)) {
     return nullptr;
   }
 
   // If we're vertically flipping the output, we need to flip the invalid rect.
   // Since we're dealing with an axis-aligned rect, only the y coordinate needs
   // to change.
--- a/image/SurfacePipe.h
+++ b/image/SurfacePipe.h
@@ -122,16 +122,29 @@ class SurfaceFilter {
    *         that we've finished the entire surface.
    */
   uint8_t* AdvanceRow() {
     mCol = 0;
     mRowPointer = DoAdvanceRow();
     return mRowPointer;
   }
 
+  /**
+   * Called by WriteBuffer() to advance this filter to the next row, if the
+   * supplied row is a full row.
+   *
+   * @return a pointer to the buffer for the next row, or nullptr to indicate
+   *         that we've finished the entire surface.
+   */
+  uint8_t* AdvanceRow(const uint8_t* aInputRow) {
+    mCol = 0;
+    mRowPointer = DoAdvanceRowFromBuffer(aInputRow);
+    return mRowPointer;
+  }
+
   /// @return a pointer to the buffer for the current row.
   uint8_t* CurrentRowPointer() const { return mRowPointer; }
 
   /// @return true if we've finished writing to the surface.
   bool IsSurfaceFinished() const { return mRowPointer == nullptr; }
 
   /// @return the input size this filter expects.
   gfx::IntSize InputSize() const { return mInputSize; }
@@ -264,17 +277,32 @@ class SurfaceFilter {
    *                null.
    *
    * @return WriteState::FINISHED if the entire surface has been written to.
    *         Otherwise, returns WriteState::NEED_MORE_DATA. If a null |aSource|
    *         value is passed, returns WriteState::FAILURE.
    */
   template <typename PixelType>
   WriteState WriteBuffer(const PixelType* aSource) {
-    return WriteBuffer(aSource, 0, mInputSize.width);
+    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 WriteState::FINISHED;  // Already done.
+    }
+
+    if (MOZ_UNLIKELY(!aSource)) {
+      NS_WARNING("Passed a null pointer to WriteBuffer");
+      return WriteState::FAILURE;
+    }
+
+    AdvanceRow(reinterpret_cast<const uint8_t*>(aSource));
+    return IsSurfaceFinished() ? WriteState::FINISHED
+                               : WriteState::NEED_MORE_DATA;
   }
 
   /**
    * Write a row to the surface by copying from a buffer. This is bounds checked
    * and memory safe with respect to the surface, but care must still be taken
    * by the caller not to overread the source buffer. This variant of
    * WriteBuffer() reads at most @aLength pixels from the buffer and writes them
    * to the row starting at @aStartColumn. Any pixels in columns before
@@ -438,16 +466,26 @@ class SurfaceFilter {
    * throw away any previously written data at this point, as all rows must be
    * written to on every pass.
    */
   virtual uint8_t* DoResetToFirstRow() = 0;
 
   /**
    * Called by AdvanceRow() to actually advance this filter to the next row.
    *
+   * @param aInputRow The input row supplied by the decoder.
+   *
+   * @return a pointer to the buffer for the next row, or nullptr to indicate
+   *         that we've finished the entire surface.
+   */
+  virtual uint8_t* DoAdvanceRowFromBuffer(const uint8_t* aInputRow) = 0;
+
+  /**
+   * Called by AdvanceRow() to actually advance this filter to the next row.
+   *
    * @return a pointer to the buffer for the next row, or nullptr to indicate
    *         that we've finished the entire surface.
    */
   virtual uint8_t* DoAdvanceRow() = 0;
 
   //////////////////////////////////////////////////////////////////////////////
   // Methods For Internal Use By Subclasses
   //////////////////////////////////////////////////////////////////////////////
@@ -465,16 +503,30 @@ class SurfaceFilter {
    */
   void ConfigureFilter(gfx::IntSize aInputSize, uint8_t aPixelSize) {
     mInputSize = aInputSize;
     mPixelSize = aPixelSize;
 
     ResetToFirstRow();
   }
 
+  /**
+   * Called by subclasses' DoAdvanceRowFromBuffer() methods to copy a decoder
+   * supplied row buffer into its internal row pointer. Ideally filters at the
+   * top of the filter pipeline are able to consume the decoder row buffer
+   * directly without the extra copy prior to performing its transformation.
+   *
+   * @param aInputRow The input row supplied by the decoder.
+   */
+  void CopyInputRow(const uint8_t* aInputRow) {
+    MOZ_ASSERT(aInputRow);
+    MOZ_ASSERT(mCol == 0);
+    memcpy(mRowPointer, aInputRow, mPixelSize * mInputSize.width);
+  }
+
  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
@@ -714,16 +766,17 @@ class AbstractSurfaceSink : public Surfa
         mImageDataLength(0),
         mRow(0),
         mFlipVertically(false) {}
 
   Maybe<SurfaceInvalidRect> TakeInvalidRect() final;
 
  protected:
   uint8_t* DoResetToFirstRow() final;
+  uint8_t* DoAdvanceRowFromBuffer(const uint8_t* aInputRow) final;
   uint8_t* DoAdvanceRow() final;
   virtual uint8_t* GetRowPointer() const = 0;
 
   gfx::IntRect
       mInvalidRect;     /// The region of the surface that has been written
                         /// to since the last call to TakeInvalidRect().
   uint8_t* mImageData;  /// A pointer to the beginning of the surface data.
   uint32_t mImageDataLength;  /// The length of the surface data.