Changeset 187121 in webkit


Ignore:
Timestamp:
Jul 21, 2015 2:19:20 PM (9 years ago)
Author:
Simon Fraser
Message:

Safari mis-applies "animation-fill-mode: forwards" when using fractional iteration count
​https://bugs.webkit.org/show_bug.cgi?id=146996

Reviewed by Dean Jackson.
Source/WebCore:

animation-fill-mode: forwards with fractional iteration counts always snapped to
1 or 0, depending on direction. Fix to compute the fill-forward state from the
correct keyframes.

If filling forwards, AnimationBase::progress() sets the elapsed time to the duration,
then uses fractionalTime() to handle animation direction mapping. If the fractionalTime
is integral, we can return early, avoiding the cost of mapping through timing functions.

Tested by existing tests.

  • page/animation/AnimationBase.cpp:

(WebCore::AnimationBase::progress):
(WebCore::AnimationBase::getElapsedTime):

  • page/animation/KeyframeAnimation.cpp:

(WebCore::KeyframeAnimation::fetchIntervalEndpointsForProperty): It was possible
to end up with prevIndex == nextIndex with reverse animations, which resulted
in divide-by-zero when computing scale. Fix by picking a nextIndex that is different
from prevIndex.

LayoutTests:

Progressions, improved tests.

  • animations/animation-direction-reverse-fill-mode-expected.txt: New results; this is a progression.
  • animations/animation-direction-reverse-fill-mode.html: Use a shorter animation. Fixed results.
  • animations/fill-mode-iteration-count-non-integer-expected.txt:
  • animations/fill-mode-iteration-count-non-integer.html: Use iteration counts that are not multiplies

of 0.5, so the test can differentiation between forward and backwards states. Add a non-linear timing
function to check that fill-forwards consults the timing functions. Don't print exact succeeding
results because they may have floating point values.

  • animations/fill-mode-reverse-expected.txt: New results; this is a progression.
  • animations/fill-mode-reverse.html: Fixed results, use gray.
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r187116 r187121  
     12015-07-21  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Safari mis-applies "animation-fill-mode: forwards" when using fractional iteration count
     4        https://bugs.webkit.org/show_bug.cgi?id=146996
     5
     6        Reviewed by Dean Jackson.
     7       
     8        Progressions, improved tests.
     9
     10        * animations/animation-direction-reverse-fill-mode-expected.txt: New results; this is a progression.
     11        * animations/animation-direction-reverse-fill-mode.html: Use a shorter animation. Fixed results.
     12        * animations/fill-mode-iteration-count-non-integer-expected.txt:
     13        * animations/fill-mode-iteration-count-non-integer.html: Use iteration counts that are not multiplies
     14        of 0.5, so the test can differentiation between forward and backwards states. Add a non-linear timing
     15        function to check that fill-forwards consults the timing functions. Don't print exact succeeding
     16        results because they may have floating point values.
     17        * animations/fill-mode-reverse-expected.txt: New results; this is a progression.
     18        * animations/fill-mode-reverse.html: Fixed results, use gray.
     19
    1202015-07-21  Said Abou-Hallawa  <sabouhallawa@apple.com>
    221

  • trunk/LayoutTests/animations/animation-direction-reverse-fill-mode-expected.txt

    r180441 r187121  
    1212PASS - end of animation - id: a expected: 100 actual: 100
    1313PASS - end of animation - id: b expected: 100 actual: 100
    14 PASS - end of animation - id: c expected: 300 actual: 300
    15 PASS - end of animation - id: d expected: 300 actual: 300
     14PASS - end of animation - id: c expected: 200 actual: 200
     15PASS - end of animation - id: d expected: 200 actual: 200
    1616PASS - end of animation - id: e expected: 300 actual: 300
    1717

  • trunk/LayoutTests/animations/animation-direction-reverse-fill-mode.html

    r180441 r187121  
    1111      width: 100px;
    1212      -webkit-animation-delay: 0.1s;
    13       -webkit-animation-duration: 2s;
     13      -webkit-animation-duration: 0.5s;
    1414      -webkit-animation-timing-function: linear;
    1515      -webkit-animation-name: anim;
    … …  
    3737    }
    3838    #e {
    39       background-color: #999;
     39      background-color: gray;
    4040      -webkit-animation-fill-mode: both;
    4141      -webkit-animation-iteration-count: 2;
    … …  
    5151      {id: "a", start: 100, end: 100},
    5252      {id: "b", start: 300, end: 100},
    53       {id: "c", start: 100, end: 300},
    54       {id: "d", start: 300, end: 300},
     53      {id: "c", start: 100, end: 200},
     54      {id: "d", start: 300, end: 200},
    5555      {id: "e", start: 300, end: 300}
    5656    ];

  • trunk/LayoutTests/animations/fill-mode-iteration-count-non-integer-expected.txt

    r110588 r187121  
    55Both
    66Both iterating
     7Both reverse alternate
     8Ease function
    79PASS - start of animation - id: a expected: 100 actual: 100
    810PASS - start of animation - id: b expected: 200 actual: 200
    … …  
    1012PASS - start of animation - id: d expected: 200 actual: 200
    1113PASS - start of animation - id: e expected: 200 actual: 200
    12 PASS - end of animation - id: a expected: 100 actual: 100
    13 PASS - end of animation - id: b expected: 100 actual: 100
    14 PASS - end of animation - id: c expected: 300 actual: 300
    15 PASS - end of animation - id: d expected: 300 actual: 300
    16 PASS - end of animation - id: e expected: 300 actual: 300
     14PASS - start of animation - id: f expected: 300 actual: 300
     15PASS - start of animation - id: g expected: 300 actual: 300
     16PASS - end of animation - id: a close to expected: 100
     17PASS - end of animation - id: b close to expected: 100
     18PASS - end of animation - id: c close to expected: 225
     19PASS - end of animation - id: d close to expected: 225
     20PASS - end of animation - id: e close to expected: 225
     21PASS - end of animation - id: f close to expected: 275
     22PASS - end of animation - id: g close to expected: 291
    1723

  • trunk/LayoutTests/animations/fill-mode-iteration-count-non-integer.html

    r119985 r187121  
    1111      left: 100px;
    1212      top: 10px;
    13       height: 100px;
     13      height: 25px;
    1414      width: 100px;
    1515      -webkit-animation-delay: 0.1s;
    16       -webkit-animation-duration: 0.2s;
     16      -webkit-animation-duration: 2s;
    1717      -webkit-animation-timing-function: linear;
    1818      -webkit-animation-name: anim;
    … …  
    3535      background-color: green;
    3636      -webkit-animation-fill-mode: forwards;
    37       -webkit-animation-iteration-count: 1.5;
     37      -webkit-animation-iteration-count: 1.25;
    3838    }
    3939    #d {
    4040      background-color: yellow;
    4141      -webkit-animation-fill-mode: both;
    42       -webkit-animation-iteration-count: 1.5;
     42      -webkit-animation-iteration-count: 1.25;
    4343    }
    4444    #e {
    45       background-color: #999;
     45      background-color: gray;
    4646      -webkit-animation-fill-mode: both;
    47       -webkit-animation-iteration-count: 2.5;
     47      -webkit-animation-iteration-count: 2.25;
    4848      -webkit-animation-direction: alternate;
     49    }
     50    #f {
     51      background-color: silver;
     52      -webkit-animation-fill-mode: both;
     53      -webkit-animation-iteration-count: 2.25;
     54      -webkit-animation-direction: alternate-reverse;
     55    }
     56    #g {
     57      background-color: orange;
     58      -webkit-animation-fill-mode: both;
     59      -webkit-animation-iteration-count: 2.25;
     60      -webkit-animation-timing-function: ease-out;
     61      -webkit-animation-direction: alternate-reverse;
    4962    }
    5063  </style>
    … …  
    5669      {id: "a", start: 100, end: 100},
    5770      {id: "b", start: 200, end: 100},
    58       {id: "c", start: 100, end: 300},
    59       {id: "d", start: 200, end: 300},
    60       {id: "e", start: 200, end: 300}
     71      {id: "c", start: 100, end: 225},
     72      {id: "d", start: 200, end: 225},
     73      {id: "e", start: 200, end: 225},
     74      {id: "f", start: 300, end: 275},
     75      {id: "g", start: 300, end: 291}
    6176    ];
    6277    var result = "";
    … …  
    8196            var realValue = window.getComputedStyle(el).getPropertyCSSValue("left").getFloatValue(CSSPrimitiveValue.CSS_NUMBER);
    8297            if (Math.abs(expectedValue - realValue) < allowance) {
    83               result += "PASS";
     98              result += "PASS - end of animation - id: " + expectedValues[i].id + " close to expected: " + expectedValue + "<br>";
    8499            } else {
    85               result += "FAIL";
     100              result += "FAIL - end of animation - id: " + expectedValues[i].id + " expected: " + expectedValue + " actual: " + realValue + "<br>";
    86101            }
    87             result += " - end of animation - id: " + expectedValues[i].id + " expected: " + expectedValue + " actual: " + realValue + "<br>";
    88102        }
    89103        document.getElementById('result').innerHTML = result;
    … …  
    129143  Both iterating
    130144</div>
     145<div id="f" class="box">
     146  Both reverse alternate
     147</div>
     148<div id="g" class="box">
     149  Ease function
     150</div>
    131151<div id="result">
    132152</div>

  • trunk/LayoutTests/animations/fill-mode-reverse-expected.txt

    r107162 r187121  
    1212PASS - end of animation - id: a expected: 100 actual: 100
    1313PASS - end of animation - id: b expected: 100 actual: 100
    14 PASS - end of animation - id: c expected: 300 actual: 300
    15 PASS - end of animation - id: d expected: 300 actual: 300
     14PASS - end of animation - id: c expected: 200 actual: 200
     15PASS - end of animation - id: d expected: 200 actual: 200
    1616PASS - end of animation - id: e expected: 200 actual: 200
    1717

  • trunk/LayoutTests/animations/fill-mode-reverse.html

    r119985 r187121  
    4040    }
    4141    #e {
    42       background-color: #999;
     42      background-color: gray;
    4343      -webkit-animation-fill-mode: both;
    4444      -webkit-animation-iteration-count: 2;
    … …  
    5353      {id: "a", start: 100, end: 100},
    5454      {id: "b", start: 300, end: 100},
    55       {id: "c", start: 100, end: 300},
    56       {id: "d", start: 300, end: 300},
     55      {id: "c", start: 100, end: 200},
     56      {id: "d", start: 300, end: 200},
    5757      {id: "e", start: 200, end: 200}
    5858    ];

  • trunk/Source/WebCore/ChangeLog

    r187120 r187121  
     12015-07-21  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Safari mis-applies "animation-fill-mode: forwards" when using fractional iteration count
     4        https://bugs.webkit.org/show_bug.cgi?id=146996
     5
     6        Reviewed by Dean Jackson.
     7
     8        animation-fill-mode: forwards with fractional iteration counts always snapped to
     9        1 or 0, depending on direction. Fix to compute the fill-forward state from the
     10        correct keyframes.
     11       
     12        If filling forwards, AnimationBase::progress() sets the elapsed time to the duration,
     13        then uses fractionalTime() to handle animation direction mapping. If the fractionalTime
     14        is integral, we can return early, avoiding the cost of mapping through timing functions.
     15
     16        Tested by existing tests.
     17
     18        * page/animation/AnimationBase.cpp:
     19        (WebCore::AnimationBase::progress):
     20        (WebCore::AnimationBase::getElapsedTime):
     21        * page/animation/KeyframeAnimation.cpp:
     22        (WebCore::KeyframeAnimation::fetchIntervalEndpointsForProperty): It was possible
     23        to end up with prevIndex == nextIndex with reverse animations, which resulted
     24        in divide-by-zero when computing scale. Fix by picking a nextIndex that is different
     25        from prevIndex.
     26
    1272015-07-21  David Hyatt  <hyatt@apple.com>
    228

  • trunk/Source/WebCore/page/animation/AnimationBase.cpp

    r184148 r187121  
    619619        return 0;
    620620
     621    if (postActive() || !m_animation->duration())
     622        return 1.0;
     623
    621624    double elapsedTime = getElapsedTime();
    622625
    … …  
    625628        duration *= m_animation->iterationCount();
    626629
    627     if (postActive() || !m_animation->duration())
    628         return 1.0;
     630    if (fillingForwards())
     631        elapsedTime = duration;
     632
     633    double fractionalTime = this->fractionalTime(scale, elapsedTime, offset);
    629634
    630635    if (m_animation->iterationCount() > 0 && elapsedTime >= duration) {
    631         const int integralIterationCount = static_cast<int>(m_animation->iterationCount());
    632         const bool iterationCountHasFractional = m_animation->iterationCount() - integralIterationCount;
    633         return (integralIterationCount % 2 || iterationCountHasFractional) ? 1.0 : 0.0;
    634     }
    635 
    636     const double fractionalTime = this->fractionalTime(scale, elapsedTime, offset);
     636        if (WTF::isIntegral(fractionalTime))
     637            return fractionalTime;
     638    }
    637639
    638640    if (!timingFunction)
    … …  
    741743    if (m_startTime <= 0)
    742744        return 0;
    743     if (postActive())
     745    if (postActive() || fillingForwards())
    744746        return 1;
    745747

  • trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp

    r184908 r187121  
    107107        prevIndex = 0;
    108108
    109     if (nextIndex == -1)
    110         nextIndex = m_keyframes.size() - 1;
     109    if (nextIndex == -1) {
     110        int lastIndex = m_keyframes.size() - 1;
     111        if (prevIndex == lastIndex)
     112            nextIndex = 0;
     113        else
     114            nextIndex = lastIndex;
     115    }
     116
     117    ASSERT(prevIndex != nextIndex);
    111118
    112119    const KeyframeValue& prevKeyframe = m_keyframes[prevIndex];
Note: See TracChangeset for help on using the changeset viewer.