Bug 1550523 - Ensure that decoding methods for frozen images request the correct frame. r=tnikkel a=jcristau
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 21 May 2019 13:34:14 -0400
changeset 536431 3e3ebfd849558d6011dec7b53ea5942ba1da8867
parent 536430 f76b9ef5c2e9f48c7838fe075d5509d0474d724a
child 536432 5d6b91a3b5365903cd7d2c48bf32f62e50bff46a
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel, jcristau
bugs1550523
milestone68.0
Bug 1550523 - Ensure that decoding methods for frozen images request the correct frame. r=tnikkel a=jcristau With WebRender, we had observed that the print preview for animated images was not displaying correctly. It should display the first frame but it was showing nothing the first time the preview was opened. Once the decoded image was available in the cache, it would display correctly if the preview was reloaded. The StartDecoding and RequestDecode variants always requested FRAME_CURRENT for animated images. They should use FRAME_FIRST for static requests / FrozenImage. Correcting this fixes the print preview. Differential Revision: https://phabricator.services.mozilla.com/D32033
image/DynamicImage.cpp
image/FrozenImage.cpp
image/FrozenImage.h
image/ImageWrapper.cpp
image/RasterImage.cpp
image/RasterImage.h
image/VectorImage.cpp
image/imgIContainer.idl
--- a/image/DynamicImage.cpp
+++ b/image/DynamicImage.cpp
@@ -214,24 +214,33 @@ DynamicImage::Draw(gfxContext* aContext,
 
   gfxUtils::DrawPixelSnapped(aContext, mDrawable, SizeDouble(drawableSize),
                              region, SurfaceFormat::B8G8R8A8, aSamplingFilter,
                              aOpacity);
   return ImgDrawResult::SUCCESS;
 }
 
 NS_IMETHODIMP
-DynamicImage::StartDecoding(uint32_t aFlags) { return NS_OK; }
+DynamicImage::StartDecoding(uint32_t aFlags, uint32_t aWhichFrame) {
+  return NS_OK;
+}
 
-bool DynamicImage::StartDecodingWithResult(uint32_t aFlags) { return true; }
+bool DynamicImage::StartDecodingWithResult(uint32_t aFlags,
+                                           uint32_t aWhichFrame) {
+  return true;
+}
 
-bool DynamicImage::RequestDecodeWithResult(uint32_t aFlags) { return true; }
+bool DynamicImage::RequestDecodeWithResult(uint32_t aFlags,
+                                           uint32_t aWhichFrame) {
+  return true;
+}
 
 NS_IMETHODIMP
-DynamicImage::RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags) {
+DynamicImage::RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags,
+                                   uint32_t aWhichFrame) {
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DynamicImage::LockImage() { return NS_OK; }
 
 NS_IMETHODIMP
 DynamicImage::UnlockImage() { return NS_OK; }
--- a/image/FrozenImage.cpp
+++ b/image/FrozenImage.cpp
@@ -87,16 +87,37 @@ FrozenImage::Draw(gfxContext* aContext, 
                   uint32_t /* aWhichFrame - ignored */,
                   SamplingFilter aSamplingFilter,
                   const Maybe<SVGImageContext>& aSVGContext, uint32_t aFlags,
                   float aOpacity) {
   return InnerImage()->Draw(aContext, aSize, aRegion, FRAME_FIRST,
                             aSamplingFilter, aSVGContext, aFlags, aOpacity);
 }
 
+NS_IMETHODIMP
+FrozenImage::StartDecoding(uint32_t aFlags, uint32_t aWhichFrame) {
+  return InnerImage()->StartDecoding(aFlags, FRAME_FIRST);
+}
+
+bool FrozenImage::StartDecodingWithResult(uint32_t aFlags,
+                                          uint32_t aWhichFrame) {
+  return InnerImage()->StartDecodingWithResult(aFlags, FRAME_FIRST);
+}
+
+bool FrozenImage::RequestDecodeWithResult(uint32_t aFlags,
+                                          uint32_t aWhichFrame) {
+  return InnerImage()->RequestDecodeWithResult(aFlags, FRAME_FIRST);
+}
+
+NS_IMETHODIMP
+FrozenImage::RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags,
+                                  uint32_t aWhichFrame) {
+  return InnerImage()->RequestDecodeForSize(aSize, aFlags, FRAME_FIRST);
+}
+
 NS_IMETHODIMP_(void)
 FrozenImage::RequestRefresh(const TimeStamp& aTime) {
   // Do nothing.
 }
 
 NS_IMETHODIMP
 FrozenImage::GetAnimationMode(uint16_t* aAnimationMode) {
   *aAnimationMode = kNormalAnimMode;
--- a/image/FrozenImage.h
+++ b/image/FrozenImage.h
@@ -54,16 +54,23 @@ class FrozenImage : public ImageWrapper 
                           const Maybe<SVGImageContext>& aSVGContext,
                           uint32_t aFlags,
                           layers::ImageContainer** aOutContainer) override;
   NS_IMETHOD_(ImgDrawResult)
   Draw(gfxContext* aContext, const nsIntSize& aSize, const ImageRegion& aRegion,
        uint32_t aWhichFrame, gfx::SamplingFilter aSamplingFilter,
        const Maybe<SVGImageContext>& aSVGContext, uint32_t aFlags,
        float aOpacity) override;
+  NS_IMETHOD StartDecoding(uint32_t aFlags, uint32_t aWhichFrame) override;
+  NS_IMETHOD_(bool)
+  StartDecodingWithResult(uint32_t aFlags, uint32_t aWhichFrame) override;
+  NS_IMETHOD_(bool)
+  RequestDecodeWithResult(uint32_t aFlags, uint32_t aWhichFrame) override;
+  NS_IMETHOD RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags,
+                                  uint32_t aWhichFrame) override;
   NS_IMETHOD_(void) RequestRefresh(const TimeStamp& aTime) override;
   NS_IMETHOD GetAnimationMode(uint16_t* aAnimationMode) override;
   NS_IMETHOD SetAnimationMode(uint16_t aAnimationMode) override;
   NS_IMETHOD ResetAnimation() override;
   NS_IMETHOD_(float) GetFrameIndex(uint32_t aWhichFrame) override;
 
  protected:
   explicit FrozenImage(Image* aImage) : ImageWrapper(aImage) {}
--- a/image/ImageWrapper.cpp
+++ b/image/ImageWrapper.cpp
@@ -188,31 +188,34 @@ ImageWrapper::Draw(gfxContext* aContext,
                    SamplingFilter aSamplingFilter,
                    const Maybe<SVGImageContext>& aSVGContext, uint32_t aFlags,
                    float aOpacity) {
   return mInnerImage->Draw(aContext, aSize, aRegion, aWhichFrame,
                            aSamplingFilter, aSVGContext, aFlags, aOpacity);
 }
 
 NS_IMETHODIMP
-ImageWrapper::StartDecoding(uint32_t aFlags) {
-  return mInnerImage->StartDecoding(aFlags);
+ImageWrapper::StartDecoding(uint32_t aFlags, uint32_t aWhichFrame) {
+  return mInnerImage->StartDecoding(aFlags, aWhichFrame);
 }
 
-bool ImageWrapper::StartDecodingWithResult(uint32_t aFlags) {
-  return mInnerImage->StartDecodingWithResult(aFlags);
+bool ImageWrapper::StartDecodingWithResult(uint32_t aFlags,
+                                           uint32_t aWhichFrame) {
+  return mInnerImage->StartDecodingWithResult(aFlags, aWhichFrame);
 }
 
-bool ImageWrapper::RequestDecodeWithResult(uint32_t aFlags) {
-  return mInnerImage->RequestDecodeWithResult(aFlags);
+bool ImageWrapper::RequestDecodeWithResult(uint32_t aFlags,
+                                           uint32_t aWhichFrame) {
+  return mInnerImage->RequestDecodeWithResult(aFlags, aWhichFrame);
 }
 
 NS_IMETHODIMP
-ImageWrapper::RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags) {
-  return mInnerImage->RequestDecodeForSize(aSize, aFlags);
+ImageWrapper::RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags,
+                                   uint32_t aWhichFrame) {
+  return mInnerImage->RequestDecodeForSize(aSize, aFlags, aWhichFrame);
 }
 
 NS_IMETHODIMP
 ImageWrapper::LockImage() {
   MOZ_ASSERT(NS_IsMainThread(),
              "Main thread to encourage serialization with UnlockImage");
   return mInnerImage->LockImage();
 }
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -1041,76 +1041,85 @@ void RasterImage::Discard() {
 
 bool RasterImage::CanDiscard() {
   return mAllSourceData &&
          // Can discard animated images if the pref is set
          (!mAnimationState || gfxPrefs::ImageMemAnimatedDiscardable());
 }
 
 NS_IMETHODIMP
-RasterImage::StartDecoding(uint32_t aFlags) {
+RasterImage::StartDecoding(uint32_t aFlags, uint32_t aWhichFrame) {
   if (mError) {
     return NS_ERROR_FAILURE;
   }
 
   if (!mHasSize) {
     mWantFullDecode = true;
     return NS_OK;
   }
 
   uint32_t flags = (aFlags & FLAG_ASYNC_NOTIFY) | FLAG_SYNC_DECODE_IF_FAST |
                    FLAG_HIGH_QUALITY_SCALING;
-  return RequestDecodeForSize(mSize, flags);
+  return RequestDecodeForSize(mSize, flags, aWhichFrame);
 }
 
-bool RasterImage::StartDecodingWithResult(uint32_t aFlags) {
+bool RasterImage::StartDecodingWithResult(uint32_t aFlags,
+                                          uint32_t aWhichFrame) {
   if (mError) {
     return false;
   }
 
   if (!mHasSize) {
     mWantFullDecode = true;
     return false;
   }
 
   uint32_t flags = (aFlags & FLAG_ASYNC_NOTIFY) | FLAG_SYNC_DECODE_IF_FAST |
                    FLAG_HIGH_QUALITY_SCALING;
-  DrawableSurface surface = RequestDecodeForSizeInternal(mSize, flags);
+  DrawableSurface surface =
+      RequestDecodeForSizeInternal(mSize, flags, aWhichFrame);
   return surface && surface->IsFinished();
 }
 
-bool RasterImage::RequestDecodeWithResult(uint32_t aFlags) {
+bool RasterImage::RequestDecodeWithResult(uint32_t aFlags,
+                                          uint32_t aWhichFrame) {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mError) {
     return false;
   }
 
   uint32_t flags = aFlags | FLAG_ASYNC_NOTIFY;
-  DrawableSurface surface = RequestDecodeForSizeInternal(mSize, flags);
+  DrawableSurface surface =
+      RequestDecodeForSizeInternal(mSize, flags, aWhichFrame);
   return surface && surface->IsFinished();
 }
 
 NS_IMETHODIMP
-RasterImage::RequestDecodeForSize(const IntSize& aSize, uint32_t aFlags) {
+RasterImage::RequestDecodeForSize(const IntSize& aSize, uint32_t aFlags,
+                                  uint32_t aWhichFrame) {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mError) {
     return NS_ERROR_FAILURE;
   }
 
-  RequestDecodeForSizeInternal(aSize, aFlags);
+  RequestDecodeForSizeInternal(aSize, aFlags, aWhichFrame);
 
   return NS_OK;
 }
 
-DrawableSurface RasterImage::RequestDecodeForSizeInternal(const IntSize& aSize,
-                                                          uint32_t aFlags) {
+DrawableSurface RasterImage::RequestDecodeForSizeInternal(
+    const IntSize& aSize, uint32_t aFlags, uint32_t aWhichFrame) {
   MOZ_ASSERT(NS_IsMainThread());
 
+  if (aWhichFrame > FRAME_MAX_VALUE) {
+    return DrawableSurface();
+  }
+
   if (mError) {
     return DrawableSurface();
   }
 
   if (!mHasSize) {
     mWantFullDecode = true;
     return DrawableSurface();
   }
@@ -1120,20 +1129,18 @@ DrawableSurface RasterImage::RequestDeco
   // redecoding an image (see bug 845147).
   bool shouldSyncDecodeIfFast =
       !mHasBeenDecoded && (aFlags & FLAG_SYNC_DECODE_IF_FAST);
 
   uint32_t flags =
       shouldSyncDecodeIfFast ? aFlags : aFlags & ~FLAG_SYNC_DECODE_IF_FAST;
 
   // Perform a frame lookup, which will implicitly start decoding if needed.
-  PlaybackType playbackType =
-      mAnimationState ? PlaybackType::eAnimated : PlaybackType::eStatic;
-  LookupResult result =
-      LookupFrame(aSize, flags, playbackType, /* aMarkUsed = */ false);
+  LookupResult result = LookupFrame(aSize, flags, ToPlaybackType(aWhichFrame),
+                                    /* aMarkUsed = */ false);
   return std::move(result.Surface());
 }
 
 static bool LaunchDecodingTask(IDecodingTask* aTask, RasterImage* aImage,
                                uint32_t aFlags, bool aHaveSourceData) {
   if (aHaveSourceData) {
     nsCString uri(aImage->GetURIString());
 
@@ -1692,17 +1699,18 @@ void RasterImage::NotifyDecodeComplete(
       NotifyForLoadEvent(*mLoadProgress);
       mLoadProgress = Nothing();
     }
 
     // If we were a metadata decode and a full decode was requested, do it.
     if (mWantFullDecode) {
       mWantFullDecode = false;
       RequestDecodeForSize(mSize,
-                           DECODE_FLAGS_DEFAULT | FLAG_HIGH_QUALITY_SCALING);
+                           DECODE_FLAGS_DEFAULT | FLAG_HIGH_QUALITY_SCALING,
+                           FRAME_CURRENT);
     }
   }
 }
 
 void RasterImage::ReportDecoderError() {
   nsCOMPtr<nsIConsoleService> consoleService =
       do_GetService(NS_CONSOLESERVICE_CONTRACTID);
   nsCOMPtr<nsIScriptError> errorObject =
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -447,17 +447,18 @@ class RasterImage final : public ImageRe
   };
 
   // Helpers
   bool CanDiscard();
 
   bool IsOpaque();
 
   DrawableSurface RequestDecodeForSizeInternal(const gfx::IntSize& aSize,
-                                               uint32_t aFlags);
+                                               uint32_t aFlags,
+                                               uint32_t aWhichFrame);
 
  protected:
   explicit RasterImage(nsIURI* aURI = nullptr);
 
   bool ShouldAnimate() override;
 
   friend class ImageFactory;
 };
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -1197,33 +1197,36 @@ void VectorImage::Show(gfxDrawable* aDra
 void VectorImage::RecoverFromLossOfSurfaces() {
   NS_WARNING("An imgFrame became invalid. Attempting to recover...");
 
   // Discard all existing frames, since they're probably all now invalid.
   SurfaceCache::RemoveImage(ImageKey(this));
 }
 
 NS_IMETHODIMP
-VectorImage::StartDecoding(uint32_t aFlags) {
+VectorImage::StartDecoding(uint32_t aFlags, uint32_t aWhichFrame) {
   // Nothing to do for SVG images
   return NS_OK;
 }
 
-bool VectorImage::StartDecodingWithResult(uint32_t aFlags) {
+bool VectorImage::StartDecodingWithResult(uint32_t aFlags,
+                                          uint32_t aWhichFrame) {
   // SVG images are ready to draw when they are loaded
   return mIsFullyLoaded;
 }
 
-bool VectorImage::RequestDecodeWithResult(uint32_t aFlags) {
+bool VectorImage::RequestDecodeWithResult(uint32_t aFlags,
+                                          uint32_t aWhichFrame) {
   // SVG images are ready to draw when they are loaded
   return mIsFullyLoaded;
 }
 
 NS_IMETHODIMP
-VectorImage::RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags) {
+VectorImage::RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags,
+                                  uint32_t aWhichFrame) {
   // Nothing to do for SVG images, though in theory we could rasterize to the
   // provided size ahead of time if we supported off-main-thread SVG
   // rasterization...
   return NS_OK;
 }
 
 //******************************************************************************
 
--- a/image/imgIContainer.idl
+++ b/image/imgIContainer.idl
@@ -462,51 +462,80 @@ interface imgIContainer : nsISupports
   /*
    * Ensures that an image is decoding. Calling this function guarantees that
    * the image will at some point fire off decode notifications. Images that
    * can be decoded "quickly" according to some heuristic will be decoded
    * synchronously.
    *
    * @param aFlags Flags of the FLAG_* variety. Only FLAG_ASYNC_NOTIFY
    *               is accepted; all others are ignored.
+   * @param aWhichFrame Frame specifier of the FRAME_* variety.
    */
-  [noscript] void startDecoding(in uint32_t aFlags);
+  [noscript] void startDecoding(in uint32_t aFlags, in uint32_t aWhichFrame);
+
+%{C++
+  nsresult StartDecoding(uint32_t aFlags) {
+    return StartDecoding(aFlags, FRAME_CURRENT);
+  }
+%}
 
   /*
    * Exactly like startDecoding above except returns whether the current frame
    * of the image is complete or not.
    *
    * @param aFlags Flags of the FLAG_* variety. Only FLAG_ASYNC_NOTIFY
    *               is accepted; all others are ignored.
+   * @param aWhichFrame Frame specifier of the FRAME_* variety.
    */
-  [noscript, notxpcom] boolean startDecodingWithResult(in uint32_t aFlags);
+  [noscript, notxpcom] boolean startDecodingWithResult(in uint32_t aFlags, in uint32_t aWhichFrame);
+
+%{C++
+  bool StartDecodingWithResult(uint32_t aFlags) {
+    return StartDecodingWithResult(aFlags, FRAME_CURRENT);
+  }
+%}
 
   /*
    * This method triggers decoding for an image, but unlike startDecoding() it
    * enables the caller to provide more detailed information about the decode
    * request.
    *
    * @param aFlags Flags of the FLAG_* variety.
+   * @param aWhichFrame Frame specifier of the FRAME_* variety.
    * @return True there is a surface that satisfies the request and it is
    *         fully decoded, else false.
    */
-  [noscript, notxpcom] boolean requestDecodeWithResult(in uint32_t aFlags);
+  [noscript, notxpcom] boolean requestDecodeWithResult(in uint32_t aFlags, in uint32_t aWhichFrame);
+
+%{C++
+  bool RequestDecodeWithResult(uint32_t aFlags) {
+    return RequestDecodeWithResult(aFlags, FRAME_CURRENT);
+  }
+%}
 
   /*
    * This method triggers decoding for an image, but unlike startDecoding() it
    * enables the caller to provide more detailed information about the decode
    * request.
    *
    * @param aSize The size to which the image should be scaled while decoding,
    *              if possible. If the image cannot be scaled to this size while
    *              being decoded, it will be decoded at its intrinsic size.
    * @param aFlags Flags of the FLAG_* variety.
+   * @param aWhichFrame Frame specifier of the FRAME_* variety.
    */
   [noscript] void requestDecodeForSize([const] in nsIntSize aSize,
-                                       in uint32_t aFlags);
+                                       in uint32_t aFlags,
+                                       in uint32_t aWhichFrame);
+
+%{C++
+  nsresult RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags) {
+    return RequestDecodeForSize(aSize, aFlags, FRAME_CURRENT);
+  }
+%}
 
   /**
     * Increments the lock count on the image. An image will not be discarded
     * as long as the lock count is nonzero. Note that it is still possible for
     * the image to be undecoded if decode-on-draw is enabled and the image
     * was never drawn.
     *
     * Upon instantiation images have a lock count of zero.