Bug 1288040 (Part 5) - Wrap frame timeout values in a FrameTimeout type that ensures they're normalized. r=edwin

This commit is contained in:
Seth Fowler 2016-07-19 16:22:34 -07:00
Родитель fc0b1f8b27
Коммит 78ccdc2e84
10 изменённых файлов: 157 добавлений и 79 удалений

Просмотреть файл

@ -411,7 +411,7 @@ Decoder::PostHasTransparency()
}
void
Decoder::PostIsAnimated(int32_t aFirstFrameTimeout)
Decoder::PostIsAnimated(FrameTimeout aFirstFrameTimeout)
{
mProgress |= FLAG_IS_ANIMATED;
mImageMetadata.SetHasAnimation();
@ -423,7 +423,8 @@ Decoder::PostFrameStop(Opacity aFrameOpacity
/* = Opacity::SOME_TRANSPARENCY */,
DisposalMethod aDisposalMethod
/* = DisposalMethod::KEEP */,
int32_t aTimeout /* = 0 */,
FrameTimeout aTimeout
/* = FrameTimeout::FromRawMilliseconds(0) */,
BlendMethod aBlendMethod /* = BlendMethod::OVER */,
const Maybe<nsIntRect>& aBlendRect /* = Nothing() */)
{

Просмотреть файл

@ -334,7 +334,7 @@ protected:
//
// @param aTimeout The time for which the first frame should be shown before
// we advance to the next frame.
void PostIsAnimated(int32_t aFirstFrameTimeout);
void PostIsAnimated(FrameTimeout aFirstFrameTimeout);
// Called by decoders when they end a frame. Informs the image, sends
// notifications, and does internal book-keeping.
@ -343,7 +343,7 @@ protected:
// this frame.
void PostFrameStop(Opacity aFrameOpacity = Opacity::SOME_TRANSPARENCY,
DisposalMethod aDisposalMethod = DisposalMethod::KEEP,
int32_t aTimeout = 0,
FrameTimeout aTimeout = FrameTimeout::FromRawMilliseconds(0),
BlendMethod aBlendMethod = BlendMethod::OVER,
const Maybe<nsIntRect>& aBlendRect = Nothing());

Просмотреть файл

@ -94,15 +94,15 @@ FrameAnimator::GetSingleLoopTime(AnimationState& aState) const
int32_t looptime = 0;
for (uint32_t i = 0; i < mImage->GetNumFrames(); ++i) {
int32_t timeout = GetTimeoutForFrame(aState, i);
if (timeout >= 0) {
looptime += static_cast<uint32_t>(timeout);
} else {
FrameTimeout timeout = GetTimeoutForFrame(aState, i);
if (timeout == FrameTimeout::Forever()) {
// If we have a frame that never times out, we're probably in an error
// case, but let's handle it more gracefully.
NS_WARNING("Negative frame timeout - how did this happen?");
NS_WARNING("Infinite frame timeout - how did this happen?");
return -1;
}
looptime += timeout.AsMilliseconds();
}
return looptime;
@ -112,22 +112,22 @@ TimeStamp
FrameAnimator::GetCurrentImgFrameEndTime(AnimationState& aState) const
{
TimeStamp currentFrameTime = aState.mCurrentAnimationFrameTime;
int32_t timeout =
FrameTimeout timeout =
GetTimeoutForFrame(aState, aState.mCurrentAnimationFrameIndex);
if (timeout < 0) {
if (timeout == FrameTimeout::Forever()) {
// We need to return a sentinel value in this case, because our logic
// doesn't work correctly if we have a negative timeout value. We use
// one year in the future as the sentinel because it works with the loop
// in RequestRefresh() below.
// doesn't work correctly if we have an infinitely long timeout. We use one
// year in the future as the sentinel because it works with the loop in
// RequestRefresh() below.
// XXX(seth): It'd be preferable to make our logic work correctly with
// negative timeouts.
// infinitely long timeouts.
return TimeStamp::NowLoRes() +
TimeDuration::FromMilliseconds(31536000.0);
}
TimeDuration durationOfTimeout =
TimeDuration::FromMilliseconds(static_cast<double>(timeout));
TimeDuration::FromMilliseconds(double(timeout.AsMilliseconds()));
TimeStamp currentFrameEndTime = currentFrameTime + durationOfTimeout;
return currentFrameEndTime;
@ -210,8 +210,7 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime)
return ret;
}
// Bad data
if (GetTimeoutForFrame(aState, nextFrameIndex) < 0) {
if (GetTimeoutForFrame(aState, nextFrameIndex) == FrameTimeout::Forever()) {
ret.animationFinished = true;
}
@ -317,40 +316,21 @@ FrameAnimator::GetCompositedFrame(uint32_t aFrameNum)
return result;
}
int32_t
FrameTimeout
FrameAnimator::GetTimeoutForFrame(AnimationState& aState, uint32_t aFrameNum) const
{
int32_t rawTimeout = 0;
RawAccessFrameRef frame = GetRawFrame(aFrameNum);
if (frame) {
AnimationData data = frame->GetAnimationData();
rawTimeout = data.mRawTimeout;
} else if (aFrameNum == 0) {
rawTimeout = aState.mFirstFrameTimeout;
} else {
NS_WARNING("No frame; called GetTimeoutForFrame too early?");
return 100;
return data.mTimeout;
}
// Ensure a minimal time between updates so we don't throttle the UI thread.
// consider 0 == unspecified and make it fast but not too fast. Unless we
// have a single loop GIF. See bug 890743, bug 125137, bug 139677, and bug
// 207059. The behavior of recent IE and Opera versions seems to be:
// IE 6/Win:
// 10 - 50ms go 100ms
// >50ms go correct speed
// Opera 7 final/Win:
// 10ms goes 100ms
// >10ms go correct speed
// It seems that there are broken tools out there that set a 0ms or 10ms
// timeout when they really want a "default" one. So munge values in that
// range.
if (rawTimeout >= 0 && rawTimeout <= 10) {
return 100;
if (aFrameNum == 0) {
return aState.mFirstFrameTimeout;
}
return rawTimeout;
NS_WARNING("No frame; called GetTimeoutForFrame too early?");
return FrameTimeout::FromRawMilliseconds(100);
}
static void

Просмотреть файл

@ -29,7 +29,7 @@ public:
: mCurrentAnimationFrameIndex(0)
, mLoopRemainingCount(-1)
, mLoopCount(-1)
, mFirstFrameTimeout(0)
, mFirstFrameTimeout(FrameTimeout::FromRawMilliseconds(0))
, mAnimationMode(aAnimationMode)
, mDoneDecoding(false)
{ }
@ -91,7 +91,7 @@ public:
* Set the timeout for the first frame. This is used to allow animation
* scheduling even before a full decode runs for this image.
*/
void SetFirstFrameTimeout(int32_t aTimeout) { mFirstFrameTimeout = aTimeout; }
void SetFirstFrameTimeout(FrameTimeout aTimeout) { mFirstFrameTimeout = aTimeout; }
private:
friend class FrameAnimator;
@ -112,7 +112,7 @@ private:
int32_t mLoopCount;
//! The timeout for the first frame of this image.
int32_t mFirstFrameTimeout;
FrameTimeout mFirstFrameTimeout;
//! The animation mode of this image. Constants defined in imgIContainer.
uint16_t mAnimationMode;
@ -181,13 +181,8 @@ public:
*/
LookupResult GetCompositedFrame(uint32_t aFrameNum);
/*
* Returns the frame's adjusted timeout. If the animation loops and the
* timeout falls in between a certain range then the timeout is adjusted so
* that it's never 0. If the animation does not loop then no adjustments are
* made.
*/
int32_t GetTimeoutForFrame(AnimationState& aState, uint32_t aFrameNum) const;
/// @return the given frame's timeout.
FrameTimeout GetTimeoutForFrame(AnimationState& aState, uint32_t aFrameNum) const;
/**
* Collect an accounting of the memory occupied by the compositing surfaces we

Просмотреть файл

@ -23,7 +23,7 @@ class ImageMetadata
public:
ImageMetadata()
: mLoopCount(-1)
, mFirstFrameTimeout(0)
, mFirstFrameTimeout(FrameTimeout::Forever())
, mHasAnimation(false)
{ }
@ -40,8 +40,8 @@ public:
}
int32_t GetLoopCount() const { return mLoopCount; }
void SetFirstFrameTimeout(int32_t aTimeout) { mFirstFrameTimeout = aTimeout; }
int32_t GetFirstFrameTimeout() const { return mFirstFrameTimeout; }
void SetFirstFrameTimeout(FrameTimeout aTimeout) { mFirstFrameTimeout = aTimeout; }
FrameTimeout GetFirstFrameTimeout() const { return mFirstFrameTimeout; }
void SetSize(int32_t width, int32_t height, Orientation orientation)
{
@ -66,7 +66,7 @@ private:
int32_t mLoopCount;
/// The timeout of an animated image's first frame.
int32_t mFirstFrameTimeout;
FrameTimeout mFirstFrameTimeout;
Maybe<nsIntSize> mSize;
Maybe<Orientation> mOrientation;

Просмотреть файл

@ -480,7 +480,8 @@ RasterImage::GetFirstFrameDelay()
MOZ_ASSERT(mAnimationState, "Animated images should have an AnimationState");
MOZ_ASSERT(mFrameAnimator, "Animated images should have a FrameAnimator");
return mFrameAnimator->GetTimeoutForFrame(*mAnimationState, 0);
return mFrameAnimator->
GetTimeoutForFrame(*mAnimationState, 0).AsEncodedValueDeprecated();
}
already_AddRefed<SourceSurface>
@ -924,10 +925,10 @@ RasterImage::StartAnimation()
MOZ_ASSERT(mFrameAnimator,
"Should have a FrameAnimator if we have AnimationState");
// If the first frame has a timeout of -1, it means we should display it
// forever. Don't bother starting to animate in this case.
// Don't bother to animate if we're displaying the first frame forever.
if (GetCurrentFrameIndex() == 0 &&
mFrameAnimator->GetTimeoutForFrame(*mAnimationState, 0) < 0) {
mFrameAnimator->GetTimeoutForFrame(*mAnimationState, 0)
== FrameTimeout::Forever()) {
mAnimationFinished = true;
return NS_ERROR_ABORT;
}

Просмотреть файл

@ -256,7 +256,7 @@ nsGIFDecoder2::EndImageFrame()
// Tell the superclass we finished a frame
PostFrameStop(opacity,
DisposalMethod(mGIFStruct.disposal_method),
mGIFStruct.delay_time);
FrameTimeout::FromRawMilliseconds(mGIFStruct.delay_time));
// Reset the transparent pixel
if (mOldColor) {
@ -695,7 +695,7 @@ nsGIFDecoder2::ReadGraphicControlExtension(const char* aData)
mGIFStruct.delay_time = LittleEndian::readUint16(aData + 1) * 10;
if (mGIFStruct.delay_time > 0) {
PostIsAnimated(mGIFStruct.delay_time);
PostIsAnimated(FrameTimeout::FromRawMilliseconds(mGIFStruct.delay_time));
}
return Transition::To(State::SKIP_SUB_BLOCKS, SUB_BLOCK_HEADER_LEN);
@ -765,7 +765,7 @@ nsGIFDecoder2::ReadImageDescriptor(const char* aData)
// animated image with a first frame timeout of zero. Signal that we're
// animated now, before the first-frame decode early exit below, so that
// RasterImage can detect that this happened.
PostIsAnimated(0);
PostIsAnimated(FrameTimeout::FromRawMilliseconds(0));
}
if (IsFirstFrameDecode()) {

Просмотреть файл

@ -262,7 +262,8 @@ nsPNGDecoder::EndImageFrame()
opacity = Opacity::FULLY_OPAQUE;
}
PostFrameStop(opacity, mAnimInfo.mDispose, mAnimInfo.mTimeout,
PostFrameStop(opacity, mAnimInfo.mDispose,
FrameTimeout::FromRawMilliseconds(mAnimInfo.mTimeout),
mAnimInfo.mBlend, Some(mFrameRect));
}
@ -675,7 +676,8 @@ nsPNGDecoder::info_callback(png_structp png_ptr, png_infop info_ptr)
#ifdef PNG_APNG_SUPPORTED
bool isAnimated = png_get_valid(png_ptr, info_ptr, PNG_INFO_acTL);
if (isAnimated) {
decoder->PostIsAnimated(GetNextFrameDelay(png_ptr, info_ptr));
int32_t rawTimeout = GetNextFrameDelay(png_ptr, info_ptr);
decoder->PostIsAnimated(FrameTimeout::FromRawMilliseconds(rawTimeout));
if (decoder->mDownscaler && !decoder->IsFirstFrameDecode()) {
MOZ_ASSERT_UNREACHABLE("Doing downscale-during-decode "

Просмотреть файл

@ -162,7 +162,7 @@ imgFrame::imgFrame()
: mMonitor("imgFrame")
, mDecoded(0, 0, 0, 0)
, mLockCount(0)
, mTimeout(100)
, mTimeout(FrameTimeout::FromRawMilliseconds(100))
, mDisposalMethod(DisposalMethod::NOT_SPECIFIED)
, mBlendMethod(BlendMethod::OVER)
, mHasNoAlpha(false)
@ -707,7 +707,8 @@ imgFrame::ImageUpdatedInternal(const nsIntRect& aUpdateRect)
void
imgFrame::Finish(Opacity aFrameOpacity /* = Opacity::SOME_TRANSPARENCY */,
DisposalMethod aDisposalMethod /* = DisposalMethod::KEEP */,
int32_t aRawTimeout /* = 0 */,
FrameTimeout aTimeout
/* = FrameTimeout::FromRawMilliseconds(0) */,
BlendMethod aBlendMethod /* = BlendMethod::OVER */,
const Maybe<IntRect>& aBlendRect /* = Nothing() */)
{
@ -719,7 +720,7 @@ imgFrame::Finish(Opacity aFrameOpacity /* = Opacity::SOME_TRANSPARENCY */,
}
mDisposalMethod = aDisposalMethod;
mTimeout = aRawTimeout;
mTimeout = aTimeout;
mBlendMethod = aBlendMethod;
mBlendRect = aBlendRect;
ImageUpdatedInternal(GetRect());

Просмотреть файл

@ -46,6 +46,106 @@ 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.
@ -57,12 +157,12 @@ enum class Opacity : uint8_t {
struct AnimationData
{
AnimationData(uint8_t* aRawData, uint32_t aPaletteDataLength,
int32_t aRawTimeout, const nsIntRect& aRect,
FrameTimeout aTimeout, const nsIntRect& aRect,
BlendMethod aBlendMethod, const Maybe<gfx::IntRect>& aBlendRect,
DisposalMethod aDisposalMethod, bool aHasAlpha)
: mRawData(aRawData)
, mPaletteDataLength(aPaletteDataLength)
, mRawTimeout(aRawTimeout)
, mTimeout(aTimeout)
, mRect(aRect)
, mBlendMethod(aBlendMethod)
, mBlendRect(aBlendRect)
@ -72,7 +172,7 @@ struct AnimationData
uint8_t* mRawData;
uint32_t mPaletteDataLength;
int32_t mRawTimeout;
FrameTimeout mTimeout;
nsIntRect mRect;
BlendMethod mBlendMethod;
Maybe<gfx::IntRect> mBlendRect;
@ -167,10 +267,8 @@ public:
* @param aDisposalMethod For animation frames, how this imgFrame is cleared
* from the compositing frame before the next frame is
* displayed.
* @param aRawTimeout For animation frames, the timeout in milliseconds
* before the next frame is displayed. This timeout is
* not necessarily the timeout that will actually be
* used; see FrameAnimator::GetTimeoutForFrame.
* @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
@ -179,7 +277,7 @@ public:
*/
void Finish(Opacity aFrameOpacity = Opacity::SOME_TRANSPARENCY,
DisposalMethod aDisposalMethod = DisposalMethod::KEEP,
int32_t aRawTimeout = 0,
FrameTimeout aTimeout = FrameTimeout::FromRawMilliseconds(0),
BlendMethod aBlendMethod = BlendMethod::OVER,
const Maybe<IntRect>& aBlendRect = Nothing());
@ -312,8 +410,8 @@ private: // data
//! Number of RawAccessFrameRefs currently alive for this imgFrame.
int32_t mLockCount;
//! Raw timeout for this frame. (See FrameAnimator::GetTimeoutForFrame.)
int32_t mTimeout; // -1 means display forever.
//! The timeout for this frame.
FrameTimeout mTimeout;
DisposalMethod mDisposalMethod;
BlendMethod mBlendMethod;