Bug 1319025 - Fix how animated images disposal method should use frame rect size instead of the image size as its bounds. r=tnikkel a=jcristau+gchang
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 25 Nov 2016 10:38:37 -0500
changeset 353047 f6a581f5ae0b6d21f642c70822b85ecee267d70f
parent 353046 99a215091429f05ac28d48b9e0cc62c80c805bea
child 353048 9e9258e6585493be7f501186f1c43305ad9900b7
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel, jcristau
bugs1319025
milestone52.0a2
Bug 1319025 - Fix how animated images disposal method should use frame rect size instead of the image size as its bounds. r=tnikkel a=jcristau+gchang
image/FrameAnimator.cpp
image/test/mochitest/bug1319025-ref.png
image/test/mochitest/bug1319025.png
image/test/mochitest/mochitest.ini
image/test/mochitest/test_animation_operators.html
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -417,33 +417,40 @@ FrameAnimator::DoBlend(IntRect* aDirtyRe
   MOZ_ASSERT(prevFrame && nextFrame, "Should have frames here");
 
   AnimationData prevFrameData = prevFrame->GetAnimationData();
   if (prevFrameData.mDisposalMethod == DisposalMethod::RESTORE_PREVIOUS &&
       !mCompositingPrevFrame) {
     prevFrameData.mDisposalMethod = DisposalMethod::CLEAR;
   }
 
-  bool isFullPrevFrame = prevFrameData.mRect.x == 0 &&
-                         prevFrameData.mRect.y == 0 &&
-                         prevFrameData.mRect.width == mSize.width &&
-                         prevFrameData.mRect.height == mSize.height;
+  IntRect prevRect = prevFrameData.mBlendRect
+                   ? prevFrameData.mRect.Intersect(*prevFrameData.mBlendRect)
+                   : prevFrameData.mRect;
+
+  bool isFullPrevFrame = prevRect.x == 0 && prevRect.y == 0 &&
+                         prevRect.width == mSize.width &&
+                         prevRect.height == mSize.height;
 
   // Optimization: DisposeClearAll if the previous frame is the same size as
   //               container and it's clearing itself
   if (isFullPrevFrame &&
       (prevFrameData.mDisposalMethod == DisposalMethod::CLEAR)) {
     prevFrameData.mDisposalMethod = DisposalMethod::CLEAR_ALL;
   }
 
   AnimationData nextFrameData = nextFrame->GetAnimationData();
-  bool isFullNextFrame = nextFrameData.mRect.x == 0 &&
-                         nextFrameData.mRect.y == 0 &&
-                         nextFrameData.mRect.width == mSize.width &&
-                         nextFrameData.mRect.height == mSize.height;
+
+  IntRect nextRect = nextFrameData.mBlendRect
+                   ? nextFrameData.mRect.Intersect(*nextFrameData.mBlendRect)
+                   : nextFrameData.mRect;
+
+  bool isFullNextFrame = nextRect.x == 0 && nextRect.y == 0 &&
+                         nextRect.width == mSize.width &&
+                         nextRect.height == mSize.height;
 
   if (!nextFrame->GetIsPaletted()) {
     // Optimization: Skip compositing if the previous frame wants to clear the
     //               whole image
     if (prevFrameData.mDisposalMethod == DisposalMethod::CLEAR_ALL) {
       aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
       return true;
     }
@@ -459,33 +466,33 @@ FrameAnimator::DoBlend(IntRect* aDirtyRe
   }
 
   // Calculate area that needs updating
   switch (prevFrameData.mDisposalMethod) {
     default:
       MOZ_FALLTHROUGH_ASSERT("Unexpected DisposalMethod");
     case DisposalMethod::NOT_SPECIFIED:
     case DisposalMethod::KEEP:
-      *aDirtyRect = nextFrameData.mRect;
+      *aDirtyRect = nextRect;
       break;
 
     case DisposalMethod::CLEAR_ALL:
       // Whole image container is cleared
       aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
       break;
 
     case DisposalMethod::CLEAR:
       // Calc area that needs to be redrawn (the combination of previous and
       // this frame)
       // XXX - This could be done with multiple framechanged calls
       //       Having prevFrame way at the top of the image, and nextFrame
       //       way at the bottom, and both frames being small, we'd be
       //       telling framechanged to refresh the whole image when only two
       //       small areas are needed.
-      aDirtyRect->UnionRect(nextFrameData.mRect, prevFrameData.mRect);
+      aDirtyRect->UnionRect(nextRect, prevRect);
       break;
 
     case DisposalMethod::RESTORE_PREVIOUS:
       aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
       break;
   }
 
   // Optimization:
@@ -531,22 +538,19 @@ FrameAnimator::DoBlend(IntRect* aDirtyRe
       nextFrameData.mDisposalMethod != DisposalMethod::RESTORE_PREVIOUS) {
     if (isFullNextFrame) {
       // Optimization: No need to dispose prev.frame when
       // next frame is full frame and not transparent.
       doDisposal = false;
       // No need to blank the composite frame
       needToBlankComposite = false;
     } else {
-      if ((prevFrameData.mRect.x >= nextFrameData.mRect.x) &&
-          (prevFrameData.mRect.y >= nextFrameData.mRect.y) &&
-          (prevFrameData.mRect.x + prevFrameData.mRect.width <=
-              nextFrameData.mRect.x + nextFrameData.mRect.width) &&
-          (prevFrameData.mRect.y + prevFrameData.mRect.height <=
-              nextFrameData.mRect.y + nextFrameData.mRect.height)) {
+      if ((prevRect.x >= nextRect.x) && (prevRect.y >= nextRect.y) &&
+          (prevRect.x + prevRect.width <= nextRect.x + nextRect.width) &&
+          (prevRect.y + prevRect.height <= nextRect.y + nextRect.height)) {
         // Optimization: No need to dispose prev.frame when
         // next frame fully overlaps previous frame.
         doDisposal = false;
       }
     }
   }
 
   if (doDisposal) {
@@ -557,17 +561,17 @@ FrameAnimator::DoBlend(IntRect* aDirtyRe
           // If we just created the composite, it could have anything in its
           // buffer. Clear whole frame
           ClearFrame(compositingFrameData.mRawData,
                      compositingFrameData.mRect);
         } else {
           // Only blank out previous frame area (both color & Mask/Alpha)
           ClearFrame(compositingFrameData.mRawData,
                      compositingFrameData.mRect,
-                     prevFrameData.mRect);
+                     prevRect);
         }
         break;
 
       case DisposalMethod::CLEAR_ALL:
         ClearFrame(compositingFrameData.mRawData,
                    compositingFrameData.mRect);
         break;
 
@@ -604,17 +608,17 @@ FrameAnimator::DoBlend(IntRect* aDirtyRe
         // smaller in size than the container, as long as the frame before it
         // erased itself)
         // Note: Frame 1 never gets into DoBlend(), so (aNextFrameIndex - 1)
         // will always be a valid frame number.
         if (mLastCompositedFrameIndex != int32_t(aNextFrameIndex - 1)) {
           if (isFullPrevFrame && !prevFrame->GetIsPaletted()) {
             // Just copy the bits
             CopyFrameImage(prevFrameData.mRawData,
-                           prevFrameData.mRect,
+                           prevRect,
                            compositingFrameData.mRawData,
                            compositingFrameData.mRect);
           } else {
             if (needToBlankComposite) {
               // Only blank composite when prev is transparent or not full.
               if (prevFrameData.mHasAlpha || !isFullPrevFrame) {
                 ClearFrame(compositingFrameData.mRawData,
                            compositingFrameData.mRect);
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..482d027a020c792d7900b24d3c38fd99b6543b0a
GIT binary patch
literal 347
zc%17D@N?(olHy`uVBq!ia0vp^DIm<j1SJ1AFfjuu#^NA%Cx&(BWL^R}Y)RhkE)4%c
zaKYZ?lYt_f1s;*b3=G`DAk4@xYmNj^kiEpy*OmPNw<N!U1n<-2_dp@p%#er@=ltB<
z)VvY~5O6L^O)N=GQ7F$W$xv|j^bJVSOJxU&Z}N0;45_&F_Vz(e21NlD$L9wBr+l9@
zqdlXPLFwqPs558Qd_Hxmar>`n2?v{4xy5uKWCVoVzyc=Y^3D0ZbB&{7{-pU#OXt9$
zDuFX>ZR{UoD>d&8V6(8AxPAKX@WpB<y1?W?Au!pb8nL&I=Q;c78FJ6BfV}1D>gTe~
HDWM4f@)c{S
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..8023e778795a99a5c9fc068c65d732165df6abf0
GIT binary patch
literal 422
zc%17D@N?(olHy`uVBq!ia0vp^DIm<j3?$ucQqzDGM`Ch_50C~4G46cT6$Yd^1AIbU
zfi%N^AY}MHUuq#pvLwhan8D%M4UjIWG>9$+n2`)Hzy)G^M{=_Q*|wf8jv*QM-kxRT
zWMJT7Ua+?Q<yH%sEe$UlJye7`U6dx`raswAJ~L7me9+(e5Xm}5AU1+xxOI#S%Z}J2
z0@-S5DUKmPnknO6a)OeiK~I+CRijm@A*WYN3)Ed4<+c?lkQK!c`d7HqJ#GI1By*XO
z%#{RVus2G#{GEklE(>9EyEh))3{)vm;u=vBoS#-wo>-L1ke-=llvt3Lu2-C<mzP>H
T?Z5B|kkdU~{an^LB{Ts5tVeAG
--- a/image/test/mochitest/mochitest.ini
+++ b/image/test/mochitest/mochitest.ini
@@ -35,16 +35,18 @@ support-files =
   bug89419.sjs
   bug900200.png
   bug900200-ref.png
   bug1132427.html
   bug1132427.gif
   bug1180105.sjs
   bug1180105-waiter.sjs
   bug1217571-iframe.html
+  bug1319025.png
+  bug1319025-ref.png
   clear.gif
   clear.png
   clear2.gif
   clear2-results.gif
   damon.jpg
   error-early.png
   filter-final.svg
   filter.svg
--- a/image/test/mochitest/test_animation_operators.html
+++ b/image/test/mochitest/test_animation_operators.html
@@ -37,16 +37,17 @@ var gTests = [
   "== keep.png green.png",
   "== restore-previous.gif green.png",
   "== restore-previous.png green.png",
 
   // Tests of the blending/compositing operators that only APNG supports.
   "== over.png grey.png",
   "!= source.png grey.png",
   "== bug900200.png bug900200-ref.png",
+  "== bug1319025.png bug1319025-ref.png",
 
   // Test of subframe updates.
   "== clear2.gif clear2-results.gif",
 ];
 
 // Maintain a reference count of how many things we're waiting for until
 // we can say the tests are done.
 var gDelayCount = 0;