Bug 1343357 - Ignore lower-priority animateMotion if a to-animation is encountered r=dholbert
authorviolet <violet.bugreport@gmail.com>
Fri, 15 Mar 2019 01:26:13 +0000
changeset 521999 8854d4a16528
parent 521998 a34bdd9a2ebc
child 522013 6149ddaa39a6
push id10870
push usernbeleuzu@mozilla.com
push dateFri, 15 Mar 2019 20:00:07 +0000
treeherdermozilla-beta@c594aee5b7a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1343357
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1343357 - Ignore lower-priority animateMotion if a to-animation is encountered r=dholbert Current impl at SVGMotionSMILType::Interpolate has some wrong assertions, it's probably caused by overlooking the special behavior of to-animation. These assumptions also lead weird animation in the product build. Now we take to-animation into account, and implement similar behavior as Chrome and Safari. Differential Revision: https://phabricator.services.mozilla.com/D23095
dom/smil/crashtests/1343357-1.html
dom/smil/crashtests/crashtests.list
dom/svg/SVGMotionSMILType.cpp
new file mode 100644
--- /dev/null
+++ b/dom/smil/crashtests/1343357-1.html
@@ -0,0 +1,12 @@
+<svg>
+  <animateMotion to="500,500"></animateMotion>
+  <animateMotion to="10,40"></animateMotion>
+</svg>
+<svg width="100" height="100">
+  <rect width="100%" height="100%" />
+  <circle r="2" fill="red">
+    <animateMotion dur="1s" from="50,50" to="80,70" additive="sum"></animateMotion>
+    <animateMotion dur="1s" from="50,50" to="80,70" additive="sum"></animateMotion>
+    <animateMotion dur="3s" to="0,80"></animateMotion>
+  </circle>
+</svg>
--- a/dom/smil/crashtests/crashtests.list
+++ b/dom/smil/crashtests/crashtests.list
@@ -50,11 +50,12 @@ load 691337-2.svg
 load 697640-1.svg
 load 699325-1.svg
 load 709907-1.svg
 load 720103-1.svg
 load 849593-1.xhtml
 load 1010681-1.svg
 load 1322770-1.svg
 load 1322849-1.svg
+load 1343357-1.html
 load 1375596-1.svg
 load 1402547-1.html
 load 1411963-1.html
--- a/dom/svg/SVGMotionSMILType.cpp
+++ b/dom/svg/SVGMotionSMILType.cpp
@@ -358,48 +358,56 @@ nsresult SVGMotionSMILType::Interpolate(
   MOZ_ASSERT(aResult.mType == this, "Unexpected result type");
   MOZ_ASSERT(aUnitDistance >= 0.0 && aUnitDistance <= 1.0,
              "unit distance value out of bounds");
 
   const MotionSegmentArray& startArr = ExtractMotionSegmentArray(aStartVal);
   const MotionSegmentArray& endArr = ExtractMotionSegmentArray(aEndVal);
   MotionSegmentArray& resultArr = ExtractMotionSegmentArray(aResult);
 
-  MOZ_ASSERT(startArr.Length() <= 1,
-             "Invalid start-point for animateMotion interpolation");
   MOZ_ASSERT(endArr.Length() == 1,
              "Invalid end-point for animateMotion interpolation");
   MOZ_ASSERT(resultArr.IsEmpty(),
              "Expecting result to be just-initialized w/ empty array");
 
   const MotionSegment& endSeg = endArr[0];
   MOZ_ASSERT(endSeg.mSegmentType == eSegmentType_PathPoint,
              "Expecting to be interpolating along a path");
 
   const PathPointParams& endParams = endSeg.mU.mPathPointParams;
-  // NOTE: path & angle should match between start & end (since presumably
-  // start & end came from the same <animateMotion> element), unless start is
-  // empty. (as it would be for pure 'to' animation)
+  // NOTE: Ususally, path & angle should match between start & end (since
+  // presumably start & end came from the same <animateMotion> element),
+  // unless start is empty. (as it would be for pure 'to' animation)
+  // Notable exception: when a to-animation layers on top of lower priority
+  // animation(s) -- see comment below.
   Path* path = endParams.mPath;
   RotateType rotateType = endSeg.mRotateType;
   float rotateAngle = endSeg.mRotateAngle;
 
   float startDist;
-  if (startArr.IsEmpty()) {
+  if (startArr.IsEmpty() ||
+      startArr[0].mU.mPathPointParams.mPath != endParams.mPath) {
+    // When a to-animation follows other lower priority animation(s),
+    // the endParams will contain a different path from the animation(s)
+    // that it layers on top of.
+    // Per SMIL spec, we should interpolate from the value at startArr.
+    // However, neither Chrome nor Safari implements to-animation that way.
+    // For simplicity, we use the same behavior as other browsers: override
+    // previous animations and start at the initial underlying value.
     startDist = 0.0f;
   } else {
+    MOZ_ASSERT(startArr.Length() <= 1,
+               "Invalid start-point for animateMotion interpolation");
     const MotionSegment& startSeg = startArr[0];
     MOZ_ASSERT(startSeg.mSegmentType == eSegmentType_PathPoint,
                "Expecting to be interpolating along a path");
     const PathPointParams& startParams = startSeg.mU.mPathPointParams;
     MOZ_ASSERT(startSeg.mRotateType == endSeg.mRotateType &&
                    startSeg.mRotateAngle == endSeg.mRotateAngle,
                "unexpected angle mismatch");
-    MOZ_ASSERT(startParams.mPath == endParams.mPath,
-               "unexpected path mismatch");
     startDist = startParams.mDistToPoint;
   }
 
   // Get the interpolated distance along our path.
   float resultDist =
       InterpolateFloat(startDist, endParams.mDistToPoint, aUnitDistance);
 
   // Construct the intermediate result segment, and put it in our outparam.