Bug 1276061 (Part 1) - Add SurfaceFilter::WriteBuffer() and SurfaceFilter::WriteEmptyRow(). r=njn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Wed, 01 Jun 2016 23:01:49 -0700
changeset 341258 48e220d82d59a695d2b89db1aa70e47e14b7a412
parent 341257 e2e57c225fd00677a1740c6546c1ef99389958dc
child 341259 306db1d372da6fb57432b737517b1bbacdb53f91
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1276061
milestone49.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 1276061 (Part 1) - Add SurfaceFilter::WriteBuffer() and SurfaceFilter::WriteEmptyRow(). r=njn
image/DownscalingFilter.h
image/SurfaceFilters.h
image/SurfacePipe.h
--- a/image/DownscalingFilter.h
+++ b/image/DownscalingFilter.h
@@ -302,23 +302,22 @@ private:
                "Writing past end of output");
 
     int32_t filterOffset = 0;
     int32_t filterLength = 0;
     MOZ_ASSERT(mOutputRow < mYFilter->num_values());
     auto filterValues =
       mYFilter->FilterForValue(mOutputRow, &filterOffset, &filterLength);
 
-    mNext.template WriteRows<uint32_t>([&](uint32_t* aRow, uint32_t aLength)
-            -> Maybe<WriteState> {
+    mNext.template WriteUnsafeComputedRow<uint32_t>([&](uint32_t* aRow,
+                                                        uint32_t aLength) {
       skia::ConvolveVertically(static_cast<const FilterValue*>(filterValues),
                                filterLength, mWindow.get(), mXFilter->num_values(),
                                reinterpret_cast<uint8_t*>(aRow), mHasAlpha,
                                supports_sse2() || supports_mmi());
-      return Some(WriteState::NEED_MORE_DATA);
     });
 
     mOutputRow++;
 
     if (mOutputRow == mNext.InputSize().height) {
       return;  // We're done.
     }
 
--- a/image/SurfaceFilters.h
+++ b/image/SurfaceFilters.h
@@ -287,25 +287,19 @@ private:
   {
     MOZ_ASSERT(aStart >= 0);
     MOZ_ASSERT(aUntil >= 0);
 
     if (aUntil <= aStart || aStart >= InputSize().height) {
       return;
     }
 
-    int32_t rowToOutput = aStart;
-    mNext.template WriteRows<PixelType>([&](PixelType* aRow, uint32_t aLength) {
-      const uint8_t* rowToOutputPointer = GetRowPointer(rowToOutput);
-      memcpy(aRow, rowToOutputPointer, aLength * sizeof(PixelType));
-
-      rowToOutput++;
-      return rowToOutput >= aUntil ? Some(WriteState::NEED_MORE_DATA)
-                                   : Nothing();
-    });
+    for (int32_t rowToOutput = aStart; rowToOutput < aUntil; ++rowToOutput) {
+      mNext.WriteBuffer(reinterpret_cast<PixelType*>(GetRowPointer(rowToOutput)));
+    }
   }
 
   uint8_t* GetRowPointer(uint32_t aRow) const
   {
     uint32_t offset = aRow * InputSize().width * sizeof(PixelType);
     MOZ_ASSERT(offset < InputSize().width * InputSize().height * sizeof(PixelType),
                "Start of row is outside of image");
     MOZ_ASSERT(offset + InputSize().width * sizeof(PixelType)
@@ -419,43 +413,35 @@ protected:
       return nullptr;
     }
 
     mRow = mUnclampedFrameRect.y;
 
     // Advance the next pipeline stage to the beginning of the frame rect,
     // outputting blank rows.
     if (mFrameRect.y > 0) {
-      int32_t rowsToWrite = mFrameRect.y;
-      mNext.template WriteRows<uint32_t>([&](uint32_t* aRow, uint32_t aLength)
-                                           -> Maybe<WriteState> {
-        memset(aRow, 0, aLength * sizeof(uint32_t));
-        rowsToWrite--;
-        return rowsToWrite > 0 ? Nothing()
-                               : Some(WriteState::NEED_MORE_DATA);
-      });
+      for (int32_t rowToOutput = 0; rowToOutput < mFrameRect.y ; ++rowToOutput) {
+        mNext.WriteEmptyRow();
+      }
     }
 
     // We're at the beginning of the frame rect now, so return if we're either
     // ready for input or we're already done.
     rowPtr = mBuffer ? mBuffer.get() : mNext.CurrentRowPointer();
     if (!mFrameRect.IsEmpty() || rowPtr == nullptr) {
       // Note that the pointer we're returning is for the next row we're
       // actually going to write to, but we may discard writes before that point
       // if mRow < mFrameRect.y.
       return AdjustRowPointer(rowPtr);
     }
 
     // We've finished the region specified by the frame rect, but the frame rect
     // is empty, so we need to output the rest of the image immediately. Advance
     // to the end of the next pipeline stage's buffer, outputting blank rows.
-    mNext.template WriteRows<uint32_t>([](uint32_t* aRow, uint32_t aLength) {
-      memset(aRow, 0, aLength * sizeof(uint32_t));
-      return Nothing();
-    });
+    while (mNext.WriteEmptyRow() == WriteState::NEED_MORE_DATA) { }
 
     mRow = mFrameRect.YMost();
     return nullptr;  // We're done.
   }
 
   uint8_t* DoAdvanceRow() override
   {
     uint8_t* rowPtr = nullptr;
@@ -469,68 +455,53 @@ protected:
       return AdjustRowPointer(rowPtr);
     } else if (currentRow >= mFrameRect.YMost()) {
       NS_WARNING("RemoveFrameRectFilter: Advancing past end of frame rect");
       return nullptr;
     }
 
     // If we had to buffer, copy the data. Otherwise, just advance the row.
     if (mBuffer) {
-      mNext.template WriteRows<uint32_t>([&](uint32_t* aRow, uint32_t aLength) {
-        // Clear the part of the row before the clamped frame rect.
-        MOZ_ASSERT(mFrameRect.x >= 0);
-        MOZ_ASSERT(uint32_t(mFrameRect.x) < aLength);
-        memset(aRow, 0, mFrameRect.x * sizeof(uint32_t));
+      // We write from the beginning of the buffer unless |mUnclampedFrameRect.x|
+      // is negative; if that's the case, we have to skip the portion of the
+      // unclamped frame rect that's outside the row.
+      uint32_t* source = reinterpret_cast<uint32_t*>(mBuffer.get()) -
+                         std::min(mUnclampedFrameRect.x, 0);
 
-        // Write the part of the row that's inside the clamped frame rect.
-        MOZ_ASSERT(mFrameRect.width >= 0);
-        aRow += mFrameRect.x;
-        aLength -= std::min(aLength, uint32_t(mFrameRect.x));
-        uint32_t toWrite = std::min(aLength, uint32_t(mFrameRect.width));
-        uint8_t* source = mBuffer.get() -
-                          std::min(mUnclampedFrameRect.x, 0) * sizeof(uint32_t);
-        MOZ_ASSERT(source >= mBuffer.get());
-        MOZ_ASSERT(source + toWrite * sizeof(uint32_t)
-                     <= mBuffer.get() + mUnclampedFrameRect.width * sizeof(uint32_t));
-        memcpy(aRow, source, toWrite * sizeof(uint32_t));
+      // We write |mFrameRect.width| columns starting at |mFrameRect.x|; we've
+      // already clamped these values to the size of the output, so we don't
+      // have to worry about bounds checking here (though WriteBuffer() will do
+      // it for us in any case).
+      WriteState state = mNext.WriteBuffer(source, mFrameRect.x, mFrameRect.width);
 
-        // Clear the part of the row after the clamped frame rect.
-        aRow += toWrite;
-        aLength -= std::min(aLength, toWrite);
-        memset(aRow, 0, aLength * sizeof(uint32_t));
-
-        return Some(WriteState::NEED_MORE_DATA);
-      });
-
-      rowPtr = mBuffer.get();
+      rowPtr = state == WriteState::NEED_MORE_DATA ? mBuffer.get()
+                                                   : nullptr;
     } else {
       rowPtr = mNext.AdvanceRow();
     }
 
-    // If there's still more data coming, just adjust the pointer and return.
+    // If there's still more data coming or we're already done, just adjust the
+    // pointer and return.
     if (mRow < mFrameRect.YMost() || rowPtr == nullptr) {
       return AdjustRowPointer(rowPtr);
     }
 
     // We've finished the region specified by the frame rect. Advance to the end
     // of the next pipeline stage's buffer, outputting blank rows.
-    mNext.template WriteRows<uint32_t>([](uint32_t* aRow, uint32_t aLength) {
-      memset(aRow, 0, aLength * sizeof(uint32_t));
-      return Nothing();
-    });
+    while (mNext.WriteEmptyRow() == WriteState::NEED_MORE_DATA) { }
 
     mRow = mFrameRect.YMost();
     return nullptr;  // We're done.
   }
 
 private:
   uint8_t* AdjustRowPointer(uint8_t* aNextRowPointer) const
   {
     if (mBuffer) {
-      MOZ_ASSERT(aNextRowPointer == mBuffer.get());
+      MOZ_ASSERT(aNextRowPointer == mBuffer.get() || aNextRowPointer == nullptr);
       return aNextRowPointer;  // No adjustment needed for an intermediate buffer.
     }
 
     if (mFrameRect.IsEmpty() ||
         mRow >= mFrameRect.YMost() ||
         aNextRowPointer == nullptr) {
       return nullptr;  // Nothing left to write.
     }
--- a/image/SurfacePipe.h
+++ b/image/SurfacePipe.h
@@ -20,16 +20,17 @@
  * is impossible as long as the SurfacePipe code is correct.
  */
 
 #ifndef mozilla_image_SurfacePipe_h
 #define mozilla_image_SurfacePipe_h
 
 #include <stdint.h>
 
+#include "mozilla/Likely.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/Move.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/unused.h"
 #include "mozilla/Variant.h"
 #include "mozilla/gfx/2D.h"
 
 namespace mozilla {
@@ -274,16 +275,195 @@ public:
         return *result;
       }
     }
 
     // We've finished the entire surface.
     return WriteState::FINISHED;
   }
 
+  /**
+   * 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() requires a source buffer which contains |mInputSize.width|
+   * pixels.
+   *
+   * 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 aSource A buffer to copy from. This buffer must be
+   *                |mInputSize.width| pixels wide,  which means
+   *                |mInputSize.width * sizeof(PixelType)| bytes. May not be
+   *                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);
+  }
+
+  /**
+   * 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
+   * @aStartColumn or after the pixels copied from the buffer are cleared.
+   *
+   * Bounds checking failures produce warnings in debug builds because although
+   * the bounds checking maintains safety, this kind of failure could indicate a
+   * bug in the calling code.
+   *
+   * 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 aSource A buffer to copy from. This buffer must be @aLength pixels
+   *                wide, which means |aLength * sizeof(PixelType)| bytes. May
+   *                not be null.
+   * @param aStartColumn The column to start writing to in the row. Columns
+   *                     before this are cleared.
+   * @param aLength The number of bytes, at most, which may be copied from
+   *                @aSource. Fewer bytes may be copied in practice due to
+   *                bounds checking.
+   *
+   * @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,
+                         const size_t aStartColumn,
+                         const size_t aLength)
+  {
+    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;
+    }
+
+    PixelType* dest = reinterpret_cast<PixelType*>(mRowPointer);
+
+    // Clear the area before |aStartColumn|.
+    const size_t prefixLength = std::min<size_t>(mInputSize.width, aStartColumn);
+    if (MOZ_UNLIKELY(prefixLength != aStartColumn)) {
+      NS_WARNING("Provided starting column is out-of-bounds in WriteBuffer");
+    }
+
+    memset(dest, 0, mInputSize.width * sizeof(PixelType));
+    dest += prefixLength;
+
+    // Write |aLength| pixels from |aSource| into the row, with bounds checking.
+    const size_t bufferLength =
+      std::min<size_t>(mInputSize.width - prefixLength, aLength);
+    if (MOZ_UNLIKELY(bufferLength != aLength)) {
+      NS_WARNING("Provided buffer length is out-of-bounds in WriteBuffer");
+    }
+
+    memcpy(dest, aSource, bufferLength * sizeof(PixelType));
+    dest += bufferLength;
+
+    // Clear the rest of the row.
+    const size_t suffixLength = mInputSize.width - (prefixLength + bufferLength);
+    memset(dest, 0, suffixLength * sizeof(PixelType));
+
+    AdvanceRow();
+
+    return IsSurfaceFinished() ? WriteState::FINISHED
+                               : WriteState::NEED_MORE_DATA;
+  }
+
+  /**
+   * Write an empty row to the surface.
+   *
+   * @return WriteState::FINISHED if the entire surface has been written to.
+   *         Otherwise, returns WriteState::NEED_MORE_DATA.
+   */
+  WriteState WriteEmptyRow()
+  {
+    if (IsSurfaceFinished()) {
+      return WriteState::FINISHED;  // Already done.
+    }
+
+    memset(mRowPointer, 0, mInputSize.width * mPixelSize);
+    AdvanceRow();
+
+    return IsSurfaceFinished() ? WriteState::FINISHED
+                               : WriteState::NEED_MORE_DATA;
+  }
+
+  /**
+   * Write a row to the surface by calling a lambda that uses a pointer to
+   * directly write to the row. This is unsafe because SurfaceFilter can't
+   * provide any bounds checking; that's up to the lambda itself. For this
+   * reason, the other Write*() methods should be preferred whenever it's
+   * possible to use them; WriteUnsafeComputedRow() should be used only when
+   * it's absolutely necessary to avoid extra copies or other performance
+   * penalties.
+   *
+   * This method should never be exposed to SurfacePipe consumers; it's strictly
+   * for use in SurfaceFilters. If external code needs this method, it should
+   * probably be turned into a SurfaceFilter.
+   *
+   * 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 writes directly to the row.
+   *
+   * @return WriteState::FINISHED if the entire surface has been written to.
+   *         Otherwise, returns WriteState::NEED_MORE_DATA.
+   */
+  template <typename PixelType, typename Func>
+  WriteState WriteUnsafeComputedRow(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 WriteState::FINISHED;  // Already done.
+    }
+
+    // Call the provided lambda with a pointer to the buffer for the current
+    // row. This is unsafe because we can't do any bounds checking; the lambda
+    // itself has to be responsible for that.
+    PixelType* rowPtr = reinterpret_cast<PixelType*>(mRowPointer);
+    aFunc(rowPtr, mInputSize.width);
+    AdvanceRow();
+
+    return IsSurfaceFinished() ? WriteState::FINISHED
+                               : WriteState::NEED_MORE_DATA;
+  }
+
   //////////////////////////////////////////////////////////////////////////////
   // Methods Subclasses Should Override
   //////////////////////////////////////////////////////////////////////////////
 
   /// @return true if this SurfaceFilter can be used with paletted surfaces.
   virtual bool IsValidPalettedPipe() const { return false; }
 
   /**
@@ -445,16 +625,59 @@ public:
    * @see SurfaceFilter::WriteRows() for the canonical documentation.
    */
   template <typename PixelType, typename Func>
   WriteState WriteRows(Func aFunc)
   {
     return mHead->WriteRows<PixelType>(Forward<Func>(aFunc));
   }
 
+  /**
+   * 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() requires a source buffer which contains |mInputSize.width|
+   * pixels.
+   *
+   * @see SurfaceFilter::WriteBuffer() for the canonical documentation.
+   */
+  template <typename PixelType>
+  WriteState WriteBuffer(const PixelType* aSource)
+  {
+    return mHead->WriteBuffer<PixelType>(aSource);
+  }
+
+  /**
+   * 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
+   * @aStartColumn or after the pixels copied from the buffer are cleared.
+   *
+   * @see SurfaceFilter::WriteBuffer() for the canonical documentation.
+   */
+  template <typename PixelType>
+  WriteState WriteBuffer(const PixelType* aSource,
+                         const size_t aStartColumn,
+                         const size_t aLength)
+  {
+    return mHead->WriteBuffer<PixelType>(aSource, aStartColumn, aLength);
+  }
+
+  /**
+   * Write an empty row to the surface.
+   *
+   * @see SurfaceFilter::WriteEmptyRow() for the canonical documentation.
+   */
+  WriteState WriteEmptyRow()
+  {
+    return mHead->WriteEmptyRow();
+  }
+
   /// @return true if we've finished writing to the surface.
   bool IsSurfaceFinished() const { return mHead->IsSurfaceFinished(); }
 
   /// @see SurfaceFilter::TakeInvalidRect() for the canonical documentation.
   Maybe<SurfaceInvalidRect> TakeInvalidRect() const
   {
     return mHead->TakeInvalidRect();
   }