Bug 1530774 - Part 3. Remove decoder support for producing paletted f…

…rames. r=tnikkel Differential Revision: https://phabricator.services.mozilla.com/D23716
dothq · Mar 18, 2019 · 3befab3 · 3befab3
1 parent 5e76ee7
commit 3befab3
Show file tree

Hide file tree

Showing 22 changed files with 161 additions and 438 deletions.
diff --git a/image/AnimationParams.h b/image/AnimationParams.h
@@ -33,6 +33,15 @@ enum class DisposalMethod : int8_t {
 };
 
 struct AnimationParams {
+  AnimationParams(const gfx::IntRect& aBlendRect, const FrameTimeout& aTimeout,
+                  uint32_t aFrameNum, BlendMethod aBlendMethod,
+                  DisposalMethod aDisposalMethod)
+      : mBlendRect(aBlendRect),
+        mTimeout(aTimeout),
+        mFrameNum(aFrameNum),
+        mBlendMethod(aBlendMethod),
+        mDisposalMethod(aDisposalMethod) {}
+
   gfx::IntRect mBlendRect;
   FrameTimeout mTimeout;
   uint32_t mFrameNum;

diff --git a/image/AnimationSurfaceProvider.cpp b/image/AnimationSurfaceProvider.cpp
@@ -31,19 +31,12 @@ AnimationSurfaceProvider::AnimationSurfaceProvider(
   MOZ_ASSERT(!mDecoder->IsFirstFrameDecode(),
              "Use DecodedSurfaceProvider for single-frame image decodes");
 
-  // We may produce paletted surfaces for GIF which means the frames are smaller
-  // than one would expect.
-  size_t pixelSize = !aDecoder->ShouldBlendAnimation() &&
-                             aDecoder->GetType() == DecoderType::GIF
-                         ? sizeof(uint8_t)
-                         : sizeof(uint32_t);
-
   // Calculate how many frames we need to decode in this animation before we
   // enter decode-on-demand mode.
   IntSize frameSize = aSurfaceKey.Size();
   size_t threshold =
       (size_t(gfxPrefs::ImageAnimatedDecodeOnDemandThresholdKB()) * 1024) /
-      (pixelSize * frameSize.width * frameSize.height);
+      (sizeof(uint32_t) * frameSize.width * frameSize.height);
   size_t batch = gfxPrefs::ImageAnimatedDecodeOnDemandBatchSize();
 
   mFrames.reset(
@@ -413,13 +406,8 @@ void AnimationSurfaceProvider::RequestFrameDiscarding() {
   auto oldFrameQueue =
       static_cast<AnimationFrameRetainedBuffer*>(mFrames.get());
 
-  // We only recycle if it is a full frame. Partial frames may be sized
-  // differently from each other. We do not support recycling with WebRender
-  // and shared surfaces at this time as there is additional synchronization
-  // required to know when it is safe to recycle.
   MOZ_ASSERT(!mDecoder->GetFrameRecycler());
-  if (gfxPrefs::ImageAnimatedDecodeOnDemandRecycle() &&
-      mDecoder->ShouldBlendAnimation()) {
+  if (gfxPrefs::ImageAnimatedDecodeOnDemandRecycle()) {
     mFrames.reset(new AnimationFrameRecyclingQueue(std::move(*oldFrameQueue)));
     mDecoder->SetFrameRecycler(this);
   } else {

diff --git a/image/Decoder.cpp b/image/Decoder.cpp
@@ -46,8 +46,6 @@ class MOZ_STACK_CLASS AutoRecordDecoderTelemetry final {
 Decoder::Decoder(RasterImage* aImage)
     : mImageData(nullptr),
       mImageDataLength(0),
-      mColormap(nullptr),
-      mColormapSize(0),
       mImage(aImage),
       mFrameRecycler(nullptr),
       mProgress(NoProgress),
@@ -253,20 +251,16 @@ DecoderTelemetry Decoder::Telemetry() const {
 }
 
 nsresult Decoder::AllocateFrame(const gfx::IntSize& aOutputSize,
-                                const gfx::IntRect& aFrameRect,
                                 gfx::SurfaceFormat aFormat,
-                                uint8_t aPaletteDepth,
                                 const Maybe<AnimationParams>& aAnimParams) {
-  mCurrentFrame =
-      AllocateFrameInternal(aOutputSize, aFrameRect, aFormat, aPaletteDepth,
-                            aAnimParams, std::move(mCurrentFrame));
+  mCurrentFrame = AllocateFrameInternal(aOutputSize, aFormat, aAnimParams,
+                                        std::move(mCurrentFrame));
 
   if (mCurrentFrame) {
     mHasFrameToTake = true;
 
     // Gather the raw pointers the decoders will use.
     mCurrentFrame->GetImageData(&mImageData, &mImageDataLength);
-    mCurrentFrame->GetPaletteData(&mColormap, &mColormapSize);
 
     // We should now be on |aFrameNum|. (Note that we're comparing the frame
     // number, which is zero-based, with the frame count, which is one-based.)
@@ -284,8 +278,7 @@ nsresult Decoder::AllocateFrame(const gfx::IntSize& aOutputSize,
 }
 
 RawAccessFrameRef Decoder::AllocateFrameInternal(
-    const gfx::IntSize& aOutputSize, const gfx::IntRect& aFrameRect,
-    SurfaceFormat aFormat, uint8_t aPaletteDepth,
+    const gfx::IntSize& aOutputSize, SurfaceFormat aFormat,
     const Maybe<AnimationParams>& aAnimParams,
     RawAccessFrameRef&& aPreviousFrame) {
   if (HasError()) {
@@ -298,8 +291,7 @@ RawAccessFrameRef Decoder::AllocateFrameInternal(
     return RawAccessFrameRef();
   }
 
-  if (aOutputSize.width <= 0 || aOutputSize.height <= 0 ||
-      aFrameRect.Width() <= 0 || aFrameRect.Height() <= 0) {
+  if (aOutputSize.width <= 0 || aOutputSize.height <= 0) {
     NS_WARNING("Trying to add frame with zero or negative size");
     return RawAccessFrameRef();
   }
@@ -310,23 +302,21 @@ RawAccessFrameRef Decoder::AllocateFrameInternal(
   }
 
   if (frameNum > 0) {
-    if (ShouldBlendAnimation()) {
-      if (aPreviousFrame->GetDisposalMethod() !=
-          DisposalMethod::RESTORE_PREVIOUS) {
-        // If the new restore frame is the direct previous frame, then we know
-        // the dirty rect is composed only of the current frame's blend rect and
-        // the restore frame's clear rect (if applicable) which are handled in
-        // filters.
-        mRestoreFrame = std::move(aPreviousFrame);
-        mRestoreDirtyRect.SetBox(0, 0, 0, 0);
-      } else {
-        // We only need the previous frame's dirty rect, because while there may
-        // have been several frames between us and mRestoreFrame, the only areas
-        // that changed are the restore frame's clear rect, the current frame
-        // blending rect, and the previous frame's blending rect. All else is
-        // forgotten due to us restoring the same frame again.
-        mRestoreDirtyRect = aPreviousFrame->GetBoundedBlendRect();
-      }
+    if (aPreviousFrame->GetDisposalMethod() !=
+        DisposalMethod::RESTORE_PREVIOUS) {
+      // If the new restore frame is the direct previous frame, then we know
+      // the dirty rect is composed only of the current frame's blend rect and
+      // the restore frame's clear rect (if applicable) which are handled in
+      // filters.
+      mRestoreFrame = std::move(aPreviousFrame);
+      mRestoreDirtyRect.SetBox(0, 0, 0, 0);
+    } else {
+      // We only need the previous frame's dirty rect, because while there may
+      // have been several frames between us and mRestoreFrame, the only areas
+      // that changed are the restore frame's clear rect, the current frame
+      // blending rect, and the previous frame's blending rect. All else is
+      // forgotten due to us restoring the same frame again.
+      mRestoreDirtyRect = aPreviousFrame->GetBoundedBlendRect();
     }
   }
 
@@ -337,10 +327,7 @@ RawAccessFrameRef Decoder::AllocateFrameInternal(
   // memory footprint, then the recycler will allow us to reuse the buffers.
   // Each frame should be the same size and have mostly the same properties.
   if (mFrameRecycler) {
-    MOZ_ASSERT(ShouldBlendAnimation());
-    MOZ_ASSERT(aPaletteDepth == 0);
     MOZ_ASSERT(aAnimParams);
-    MOZ_ASSERT(aFrameRect.IsEqualEdges(IntRect(IntPoint(0, 0), aOutputSize)));
 
     ref = mFrameRecycler->RecycleFrame(mRecycleRect);
     if (ref) {
@@ -368,9 +355,8 @@ RawAccessFrameRef Decoder::AllocateFrameInternal(
 
     bool nonPremult = bool(mSurfaceFlags & SurfaceFlags::NO_PREMULTIPLY_ALPHA);
     auto frame = MakeNotNull<RefPtr<imgFrame>>();
-    if (NS_FAILED(frame->InitForDecoder(
-            aOutputSize, aFrameRect, aFormat, aPaletteDepth, nonPremult,
-            aAnimParams, ShouldBlendAnimation(), bool(mFrameRecycler)))) {
+    if (NS_FAILED(frame->InitForDecoder(aOutputSize, aFormat, nonPremult,
+                                        aAnimParams, bool(mFrameRecycler)))) {
       NS_WARNING("imgFrame::Init should succeed");
       return RawAccessFrameRef();
     }

diff --git a/image/Decoder.h b/image/Decoder.h
@@ -271,14 +271,6 @@ class Decoder {
     return bool(mDecoderFlags & DecoderFlags::FIRST_FRAME_ONLY);
   }
 
-  /**
-   * Should blend the current frame with the previous frames to produce a
-   * complete frame instead of a partial frame for animated images.
-   */
-  bool ShouldBlendAnimation() const {
-    return bool(mDecoderFlags & DecoderFlags::BLEND_ANIMATION);
-  }
-
   /**
    * @return the number of complete animation frames which have been decoded so
    * far, if it has changed since the last call to TakeCompleteFrameCount();
@@ -418,20 +410,11 @@ class Decoder {
    * For use during decoding only. Allows the BlendAnimationFilter to get the
    * frame it should be pulling the previous frame data from.
    */
-  const RawAccessFrameRef& GetRestoreFrameRef() const {
-    MOZ_ASSERT(ShouldBlendAnimation());
-    return mRestoreFrame;
-  }
+  const RawAccessFrameRef& GetRestoreFrameRef() const { return mRestoreFrame; }
 
-  const gfx::IntRect& GetRestoreDirtyRect() const {
-    MOZ_ASSERT(ShouldBlendAnimation());
-    return mRestoreDirtyRect;
-  }
+  const gfx::IntRect& GetRestoreDirtyRect() const { return mRestoreDirtyRect; }
 
-  const gfx::IntRect& GetRecycleRect() const {
-    MOZ_ASSERT(ShouldBlendAnimation());
-    return mRecycleRect;
-  }
+  const gfx::IntRect& GetRecycleRect() const { return mRecycleRect; }
 
   const gfx::IntRect& GetFirstFrameRefreshArea() const {
     return mFirstFrameRefreshArea;
@@ -542,12 +525,9 @@ class Decoder {
 
   /**
    * Allocates a new frame, making it our current frame if successful.
-   *
-   * If a non-paletted frame is desired, pass 0 for aPaletteDepth.
    */
   nsresult AllocateFrame(const gfx::IntSize& aOutputSize,
-                         const gfx::IntRect& aFrameRect,
-                         gfx::SurfaceFormat aFormat, uint8_t aPaletteDepth = 0,
+                         gfx::SurfaceFormat aFormat,
                          const Maybe<AnimationParams>& aAnimParams = Nothing());
 
  private:
@@ -572,18 +552,15 @@ class Decoder {
   }
 
   RawAccessFrameRef AllocateFrameInternal(
-      const gfx::IntSize& aOutputSize, const gfx::IntRect& aFrameRect,
-      gfx::SurfaceFormat aFormat, uint8_t aPaletteDepth,
+      const gfx::IntSize& aOutputSize, gfx::SurfaceFormat aFormat,
       const Maybe<AnimationParams>& aAnimParams,
       RawAccessFrameRef&& aPreviousFrame);
 
  protected:
   Maybe<Downscaler> mDownscaler;
 
-  uint8_t* mImageData;  // Pointer to image data in either Cairo or 8bit format
+  uint8_t* mImageData;  // Pointer to image data in BGRA/X
   uint32_t mImageDataLength;
-  uint32_t* mColormap;  // Current colormap to be used in Cairo format
-  uint32_t mColormapSize;
 
  private:
   RefPtr<RasterImage> mImage;

diff --git a/image/DecoderFlags.h b/image/DecoderFlags.h
@@ -31,14 +31,6 @@ enum class DecoderFlags : uint8_t {
    * set.
    */
   CANNOT_SUBSTITUTE = 1 << 4,
-
-  /**
-   * By default, an animation decoder will produce partial frames that need to
-   * be combined with the previously displayed/composited frame by FrameAnimator
-   * to produce a complete frame. If this flag is set, the decoder will perform
-   * this blending at decode time, and the frames produced are complete.
-   */
-  BLEND_ANIMATION = 1 << 5
 };
 MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(DecoderFlags)
 

diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp
@@ -1196,8 +1196,6 @@ bool RasterImage::Decode(const IntSize& aSize, uint32_t aFlags,
   nsresult rv;
   bool animated = mAnimationState && aPlaybackType == PlaybackType::eAnimated;
   if (animated) {
-    decoderFlags |= DecoderFlags::BLEND_ANIMATION;
-
     size_t currentFrame = mAnimationState->GetCurrentAnimationFrameIndex();
     rv = DecoderFactory::CreateAnimationDecoder(
         mDecoderType, WrapNotNull(this), mSourceBuffer, mSize, decoderFlags,
@@ -1348,7 +1346,7 @@ ImgDrawResult RasterImage::DrawInternal(DrawableSurface&& aSurface,
 
   // By now we may have a frame with the requested size. If not, we need to
   // adjust the drawing parameters accordingly.
-  IntSize finalSize = aSurface->GetImageSize();
+  IntSize finalSize = aSurface->GetSize();
   bool couldRedecodeForBetterFrame = false;
   if (finalSize != aSize) {
     gfx::Size scale(double(aSize.width) / finalSize.width,

diff --git a/image/SurfaceFilters.h b/image/SurfaceFilters.h
@@ -360,11 +360,6 @@ class BlendAnimationFilter final : public SurfaceFilter {
       return rv;
     }
 
-    if (!aConfig.mDecoder || !aConfig.mDecoder->ShouldBlendAnimation()) {
-      MOZ_ASSERT_UNREACHABLE("Expected image decoder that is blending!");
-      return NS_ERROR_INVALID_ARG;
-    }
-
     imgFrame* currentFrame = aConfig.mDecoder->GetCurrentFrame();
     if (!currentFrame) {
       MOZ_ASSERT_UNREACHABLE("Decoder must have current frame!");
@@ -418,7 +413,7 @@ class BlendAnimationFilter final : public SurfaceFilter {
       const RawAccessFrameRef& restoreFrame =
           aConfig.mDecoder->GetRestoreFrameRef();
       if (restoreFrame) {
-        MOZ_ASSERT(restoreFrame->GetImageSize() == outputSize);
+        MOZ_ASSERT(restoreFrame->GetSize() == outputSize);
         MOZ_ASSERT(restoreFrame->IsFinished());
 
         // We can safely use this pointer without holding a RawAccessFrameRef

diff --git a/image/SurfacePipe.cpp b/image/SurfacePipe.cpp
@@ -54,20 +54,14 @@ uint8_t* AbstractSurfaceSink::DoAdvanceRow() {
 }
 
 nsresult SurfaceSink::Configure(const SurfaceConfig& aConfig) {
-  // For non-paletted surfaces, the surface size is just the output size.
   IntSize surfaceSize = aConfig.mOutputSize;
 
-  // Non-paletted surfaces should not have frame rects, so we just pass
-  // AllocateFrame() a frame rect which covers the entire surface.
-  IntRect frameRect(0, 0, surfaceSize.width, surfaceSize.height);
-
   // Allocate the frame.
   // XXX(seth): Once every Decoder subclass uses SurfacePipe, we probably want
   // to allocate the frame directly here and get rid of Decoder::AllocateFrame
   // altogether.
-  nsresult rv = aConfig.mDecoder->AllocateFrame(
-      surfaceSize, frameRect, aConfig.mFormat,
-      /* aPaletteDepth */ 0, aConfig.mAnimParams);
+  nsresult rv = aConfig.mDecoder->AllocateFrame(surfaceSize, aConfig.mFormat,
+                                                aConfig.mAnimParams);
   if (NS_FAILED(rv)) {
     return rv;
   }

diff --git a/image/SurfacePipeFactory.h b/image/SurfacePipeFactory.h
@@ -59,10 +59,6 @@ enum class SurfacePipeFlags {
                                  // result in a better user experience for
                                  // progressive display but which may be more
                                  // computationally expensive.
-
-  BLEND_ANIMATION = 1 << 4  // If set, produce the next full frame of an
-                            // animation instead of a partial frame to be
-                            // blended later.
 };
 MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(SurfacePipeFlags)
 
@@ -102,8 +98,7 @@ class SurfacePipeFactory {
     const bool downscale = aInputSize != aOutputSize;
     const bool removeFrameRect = !aFrameRect.IsEqualEdges(
         nsIntRect(0, 0, aInputSize.width, aInputSize.height));
-    const bool blendAnimation =
-        bool(aFlags & SurfacePipeFlags::BLEND_ANIMATION);
+    const bool blendAnimation = aAnimParams.isSome();
 
     // Don't interpolate if we're sure we won't show this surface to the user
     // until it's completely decoded. The final pass of an ADAM7 image doesn't
@@ -118,8 +113,6 @@ class SurfacePipeFactory {
       return Nothing();
     }
 
-    MOZ_ASSERT_IF(blendAnimation, aAnimParams);
-
     // Construct configurations for the SurfaceFilters. Note that the order of
     // these filters is significant. We want to deinterlace or interpolate raw
     // input rows, before any other transformations, and we want to remove the

diff --git a/image/decoders/nsBMPDecoder.cpp b/image/decoders/nsBMPDecoder.cpp
@@ -657,9 +657,9 @@ LexerTransition<nsBMPDecoder::State> nsBMPDecoder::ReadBitfields(
   }
 
   MOZ_ASSERT(!mImageData, "Already have a buffer allocated?");
-  nsresult rv = AllocateFrame(
-      OutputSize(), FullOutputFrame(),
-      mMayHaveTransparency ? SurfaceFormat::B8G8R8A8 : SurfaceFormat::B8G8R8X8);
+  nsresult rv = AllocateFrame(OutputSize(), mMayHaveTransparency
+                                                ? SurfaceFormat::B8G8R8A8
+                                                : SurfaceFormat::B8G8R8X8);
   if (NS_FAILED(rv)) {
     return Transition::TerminateFailure();
   }