From 807acf738f63d95fbecfb9129092aaa2379889ba Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Tue, 20 Nov 2018 20:38:49 +0100 Subject: Bug 1462355 - Part 1a. Make imgFrame animation parameters threadsafe. We currently choose to set the animation parameters (blend method, blend rect, disposal method, timeout) in imgFrame::Finish instead of imgFrame::InitForDecoder. The decoders themselves already have access to the necessary information at the time InitForDecoder is called, so there is no reason to do this. Moving the configuration to initialization will allow us to relax the mutex protection on these parameters. This part simply reorganizes imgFrame, and subsequent parts will introduce the necessary changes to SurfacePipe and decoders. --- image/imgFrame.h | 72 ++++++++++++++++++++------------------------------------ 1 file changed, 26 insertions(+), 46 deletions(-) (limited to 'image/imgFrame.h') diff --git a/image/imgFrame.h b/image/imgFrame.h index e864aca7f..32aabacae 100644 --- a/image/imgFrame.h +++ b/image/imgFrame.h @@ -12,6 +12,7 @@ #include "mozilla/Monitor.h" #include "mozilla/Move.h" #include "mozilla/VolatileBuffer.h" +#include "AnimationParams.h" #include "gfxDrawable.h" #include "imgIContainer.h" #include "MainThreadUtils.h" @@ -23,24 +24,6 @@ class ImageRegion; class DrawableFrameRef; class RawAccessFrameRef; -enum class BlendMethod : int8_t { - // All color components of the frame, including alpha, overwrite the current - // contents of the frame's output buffer region. - SOURCE, - - // The frame should be composited onto the output buffer based on its alpha, - // using a simple OVER operation. - OVER -}; - -enum class DisposalMethod : int8_t { - CLEAR_ALL = -1, // Clear the whole image, revealing what's underneath. - NOT_SPECIFIED, // Leave the frame and let the new frame draw on top. - KEEP, // Leave the frame and let the new frame draw on top. - CLEAR, // Clear the frame's area, revealing what's underneath. - RESTORE_PREVIOUS // Restore the previous (composited) frame. -}; - enum class Opacity : uint8_t { FULLY_OPAQUE, SOME_TRANSPARENCY @@ -210,14 +193,19 @@ public: const nsIntRect& aRect, SurfaceFormat aFormat, uint8_t aPaletteDepth = 0, - bool aNonPremult = false); + bool aNonPremult = false, + const Maybe& aAnimParams = Nothing()); nsresult InitForDecoder(const nsIntSize& aSize, SurfaceFormat aFormat, uint8_t aPaletteDepth = 0) { - return InitForDecoder(aSize, nsIntRect(0, 0, aSize.width, aSize.height), - aFormat, aPaletteDepth); + nsIntRect frameRect(0, 0, aSize.width, aSize.height); + AnimationParams animParams { frameRect, FrameTimeout::Forever(), + /* aFrameNum */ 1, BlendMethod::OVER, + DisposalMethod::NOT_SPECIFIED }; + return InitForDecoder(aSize, frameRect, + aFormat, aPaletteDepth, false, Some(animParams)); } @@ -268,22 +256,8 @@ public: * RawAccessFrameRef pointing to an imgFrame. * * @param aFrameOpacity Whether this imgFrame is opaque. - * @param aDisposalMethod For animation frames, how this imgFrame is cleared - * from the compositing frame before the next frame is - * displayed. - * @param aTimeout For animation frames, the timeout before the next - * frame is displayed. - * @param aBlendMethod For animation frames, a blending method to be used - * when compositing this frame. - * @param aBlendRect For animation frames, if present, the subrect in - * which @aBlendMethod applies. Outside of this - * subrect, BlendMethod::OVER is always used. */ - void Finish(Opacity aFrameOpacity = Opacity::SOME_TRANSPARENCY, - DisposalMethod aDisposalMethod = DisposalMethod::KEEP, - FrameTimeout aTimeout = FrameTimeout::FromRawMilliseconds(0), - BlendMethod aBlendMethod = BlendMethod::OVER, - const Maybe& aBlendRect = Nothing()); + void Finish(Opacity aFrameOpacity = Opacity::SOME_TRANSPARENCY); /** * Mark this imgFrame as aborted. This informs the imgFrame that if it isn't @@ -322,9 +296,15 @@ public: */ uint32_t GetBytesPerPixel() const { return GetIsPaletted() ? 1 : 4; } - IntSize GetImageSize() const { return mImageSize; } - IntRect GetRect() const { return mFrameRect; } + const IntSize& GetImageSize() const { return mImageSize; } + const IntRect& GetRect() const { return mFrameRect; } IntSize GetSize() const { return mFrameRect.Size(); } + const IntRect& GetBlendRect() const { return mBlendRect; } + IntRect GetBoundedBlendRect() const { return mBlendRect.Intersect(mFrameRect); } + FrameTimeout GetTimeout() const { return mTimeout; } + BlendMethod GetBlendMethod() const { return mBlendMethod; } + DisposalMethod GetDisposalMethod() const { return mDisposalMethod; } + bool FormatHasAlpha() const { return mFormat == SurfaceFormat::B8G8R8A8; } void GetImageData(uint8_t** aData, uint32_t* length) const; uint8_t* GetImageData() const; @@ -406,14 +386,6 @@ private: // data //! Number of RawAccessFrameRefs currently alive for this imgFrame. int32_t mLockCount; - //! The timeout for this frame. - FrameTimeout mTimeout; - - DisposalMethod mDisposalMethod; - BlendMethod mBlendMethod; - Maybe mBlendRect; - SurfaceFormat mFormat; - bool mHasNoAlpha; bool mAborted; bool mFinished; @@ -426,6 +398,14 @@ private: // data IntSize mImageSize; IntRect mFrameRect; + IntRect mBlendRect; + + //! The timeout for this frame. + FrameTimeout mTimeout; + + DisposalMethod mDisposalMethod; + BlendMethod mBlendMethod; + SurfaceFormat mFormat; // The palette and image data for images that are paletted, since Cairo // doesn't support these images. -- cgit v1.2.3 From a51993521f26a0175c6b474f6cad30eb8f8a0f86 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Wed, 21 Nov 2018 14:08:47 +0100 Subject: Split out FrameTimeout into its own header file for re-use. --- image/imgFrame.h | 100 ------------------------------------------------------- 1 file changed, 100 deletions(-) (limited to 'image/imgFrame.h') diff --git a/image/imgFrame.h b/image/imgFrame.h index 32aabacae..928f6ad86 100644 --- a/image/imgFrame.h +++ b/image/imgFrame.h @@ -29,106 +29,6 @@ enum class Opacity : uint8_t { SOME_TRANSPARENCY }; -/** - * FrameTimeout wraps a frame timeout value (measured in milliseconds) after - * first normalizing it. This normalization is necessary because some tools - * generate incorrect frame timeout values which we nevertheless have to - * support. For this reason, code that deals with frame timeouts should always - * use a FrameTimeout value rather than the raw value from the image header. - */ -struct FrameTimeout -{ - /** - * @return a FrameTimeout of zero. This should be used only for math - * involving FrameTimeout values. You can't obtain a zero FrameTimeout from - * FromRawMilliseconds(). - */ - static FrameTimeout Zero() { return FrameTimeout(0); } - - /// @return an infinite FrameTimeout. - static FrameTimeout Forever() { return FrameTimeout(-1); } - - /// @return a FrameTimeout obtained by normalizing a raw timeout value. - static FrameTimeout FromRawMilliseconds(int32_t aRawMilliseconds) - { - // Normalize all infinite timeouts to the same value. - if (aRawMilliseconds < 0) { - return FrameTimeout::Forever(); - } - - // Very small timeout values are problematic for two reasons: we don't want - // to burn energy redrawing animated images extremely fast, and broken tools - // generate these values when they actually want a "default" value, so such - // images won't play back right without normalization. For some context, - // see bug 890743, bug 125137, bug 139677, and bug 207059. The historical - // behavior of IE and Opera was: - // IE 6/Win: - // 10 - 50ms is normalized to 100ms. - // >50ms is used unnormalized. - // Opera 7 final/Win: - // 10ms is normalized to 100ms. - // >10ms is used unnormalized. - if (aRawMilliseconds >= 0 && aRawMilliseconds <= 10 ) { - return FrameTimeout(100); - } - - // The provided timeout value is OK as-is. - return FrameTimeout(aRawMilliseconds); - } - - bool operator==(const FrameTimeout& aOther) const - { - return mTimeout == aOther.mTimeout; - } - - bool operator!=(const FrameTimeout& aOther) const { return !(*this == aOther); } - - FrameTimeout operator+(const FrameTimeout& aOther) - { - if (*this == Forever() || aOther == Forever()) { - return Forever(); - } - - return FrameTimeout(mTimeout + aOther.mTimeout); - } - - FrameTimeout& operator+=(const FrameTimeout& aOther) - { - *this = *this + aOther; - return *this; - } - - /** - * @return this FrameTimeout's value in milliseconds. Illegal to call on a - * an infinite FrameTimeout value. - */ - uint32_t AsMilliseconds() const - { - if (*this == Forever()) { - MOZ_ASSERT_UNREACHABLE("Calling AsMilliseconds() on an infinite FrameTimeout"); - return 100; // Fail to something sane. - } - - return uint32_t(mTimeout); - } - - /** - * @return this FrameTimeout value encoded so that non-negative values - * represent a timeout in milliseconds, and -1 represents an infinite - * timeout. - * - * XXX(seth): This is a backwards compatibility hack that should be removed. - */ - int32_t AsEncodedValueDeprecated() const { return mTimeout; } - -private: - explicit FrameTimeout(int32_t aTimeout) - : mTimeout(aTimeout) - { } - - int32_t mTimeout; -}; - /** * AnimationData contains all of the information necessary for using an imgFrame * as part of an animation. -- cgit v1.2.3