⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 275868 in webkit


Ignore:
Timestamp:
Apr 13, 2021, 1:05:24 AM (6 years ago)
Author:
Said Abou-Hallawa
Message:

SVG paced value animations overwrite user-provided keyTimes
​https://bugs.webkit.org/show_bug.cgi?id=109010

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

  • web-platform-tests/svg/animations/scripted/paced-value-animation-overwrites-keyTimes-expected.txt:

Source/WebCore:

If the calcMode is Paced, the 'keyTimes' attribute is ignored. Distances
between the 'values' are used produce an even pace of change across the
animation.

When changing calcMode, times defined in the 'keyTimes' attribute should
be used instead. To fix this, SVGAnimationElement can maintain two lists
for keyTimes: (1) keyTimesFromAttribute (2) keyTimesForPaced.
One of these lists will be picked by a new function 'keyTimes()' based
on the current calcMode.

Specs: ​https://www.w3.org/TR/SVG11/animate.html#CalcModeAttribute

Test: svg/animations/animate-calcMode-paced-overwrite-key-times.html

  • svg/SVGAnimationElement.cpp:

(WebCore::SVGAnimationElement::parseAttribute):
(WebCore::SVGAnimationElement::calculateKeyTimesForCalcModePaced):
(WebCore::SVGAnimationElement::keyTimes const):
(WebCore::SVGAnimationElement::calculateKeyTimesIndex const):
(WebCore::SVGAnimationElement::calculatePercentFromKeyPoints const):
(WebCore::SVGAnimationElement::calculatePercentForFromTo const):
(WebCore::SVGAnimationElement::currentValuesFromKeyPoints const):
(WebCore::SVGAnimationElement::currentValuesForValuesAnimation):
(WebCore::SVGAnimationElement::startedActiveInterval):
(WebCore::SVGAnimationElement::updateAnimation):

  • svg/SVGAnimationElement.h:

LayoutTests:

Simplified from the WPT paced-value-animation-overwrites-keyTimes.html.

  • svg/animations/animate-calcMode-paced-overwrite-key-times-expected.html: Added.
  • svg/animations/animate-calcMode-paced-overwrite-key-times.html: Added.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r275865 r275868  
     12021-04-13  Said Abou-Hallawa  <said@apple.com>
     2
     3        SVG paced value animations overwrite user-provided keyTimes
     4        https://bugs.webkit.org/show_bug.cgi?id=109010
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Simplified from the WPT paced-value-animation-overwrites-keyTimes.html.
     9
     10        * svg/animations/animate-calcMode-paced-overwrite-key-times-expected.html: Added.
     11        * svg/animations/animate-calcMode-paced-overwrite-key-times.html: Added.
     12
    1132021-04-12  Xabier Rodriguez Calvar  <calvaris@igalia.com>
    214

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r275855 r275868  
     12021-04-13  Said Abou-Hallawa  <said@apple.com>
     2
     3        SVG paced value animations overwrite user-provided keyTimes
     4        https://bugs.webkit.org/show_bug.cgi?id=109010
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        * web-platform-tests/svg/animations/scripted/paced-value-animation-overwrites-keyTimes-expected.txt:
     9
    1102021-04-12  Commit Queue  <commit-queue@webkit.org>
    211

  • trunk/LayoutTests/imported/w3c/web-platform-tests/svg/animations/scripted/paced-value-animation-overwrites-keyTimes-expected.txt

    r250276 r275868  
    11
    2 FAIL Paced value animation doesn't overwrite keyTimes assert_approx_equals: expected 150 +/- 5 but got 120
     2PASS Paced value animation doesn't overwrite keyTimes
    33

  • trunk/Source/WebCore/ChangeLog

    r275859 r275868  
     12021-04-13  Said Abou-Hallawa  <said@apple.com>
     2
     3        SVG paced value animations overwrite user-provided keyTimes
     4        https://bugs.webkit.org/show_bug.cgi?id=109010
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        If the calcMode is Paced, the 'keyTimes' attribute is ignored. Distances
     9        between the 'values' are used produce an even pace of change across the
     10        animation.
     11
     12        When changing calcMode, times defined in the 'keyTimes' attribute should
     13        be used instead. To fix this, SVGAnimationElement can maintain two lists
     14        for keyTimes: (1) keyTimesFromAttribute (2) keyTimesForPaced.
     15        One of these lists will be picked by a new function 'keyTimes()' based
     16        on the current calcMode.
     17
     18        Specs: https://www.w3.org/TR/SVG11/animate.html#CalcModeAttribute
     19
     20        Test: svg/animations/animate-calcMode-paced-overwrite-key-times.html
     21
     22        * svg/SVGAnimationElement.cpp:
     23        (WebCore::SVGAnimationElement::parseAttribute):
     24        (WebCore::SVGAnimationElement::calculateKeyTimesForCalcModePaced):
     25        (WebCore::SVGAnimationElement::keyTimes const):
     26        (WebCore::SVGAnimationElement::calculateKeyTimesIndex const):
     27        (WebCore::SVGAnimationElement::calculatePercentFromKeyPoints const):
     28        (WebCore::SVGAnimationElement::calculatePercentForFromTo const):
     29        (WebCore::SVGAnimationElement::currentValuesFromKeyPoints const):
     30        (WebCore::SVGAnimationElement::currentValuesForValuesAnimation):
     31        (WebCore::SVGAnimationElement::startedActiveInterval):
     32        (WebCore::SVGAnimationElement::updateAnimation):
     33        * svg/SVGAnimationElement.h:
     34
    1352021-04-12  Ryosuke Niwa  <rniwa@webkit.org>
    236

  • trunk/Source/WebCore/svg/SVGAnimationElement.cpp

    r275410 r275868  
    33 * Copyright (C) 2004, 2005, 2006, 2007 Rob Buis <buis@kde.org>
    44 * Copyright (C) 2007 Eric Seidel <eric@webkit.org>
    5  * Copyright (C) 2008, 2017 Apple Inc. All rights reserved.
     5 * Copyright (C) 2008-2021 Apple Inc. All rights reserved.
    66 * Copyright (C) 2009 Cameron McCormack <cam@mcc.id.au>
    77 * Copyright (C) Research In Motion Limited 2010. All rights reserved.
    … …  
    162162
    163163    if (name == SVGNames::keyTimesAttr) {
    164         parseKeyTimes(value, m_keyTimes, true);
     164        parseKeyTimes(value, m_keyTimesFromAttribute, true);
    165165        return;
    166166    }
    … …  
    347347        return;
    348348
    349     // FIXME, webkit.org/b/109010: m_keyTimes should not be modified in this function.
    350     m_keyTimes.clear();
     349    m_keyTimesForPaced.clear();
    351350
    352351    Vector<float> keyTimesForPaced;
    … …  
    369368    keyTimesForPaced[keyTimesForPaced.size() - 1] = 1;
    370369
    371     // Use key times calculated based on pacing instead of the user provided ones.
    372     m_keyTimes = keyTimesForPaced;
     370    m_keyTimesForPaced = WTFMove(keyTimesForPaced);
    373371}
    374372
    375373static inline double solveEpsilon(double duration) { return 1 / (200 * duration); }
    376374
     375const Vector<float>& SVGAnimationElement::keyTimes() const
     376{
     377    return calcMode() == CalcMode::Paced ? m_keyTimesForPaced : m_keyTimesFromAttribute;
     378}
     379
    377380unsigned SVGAnimationElement::calculateKeyTimesIndex(float percent) const
    378381{
     382    const auto& keyTimes = this->keyTimes();
    379383    unsigned index;
    380     unsigned keyTimesCount = m_keyTimes.size();
     384    unsigned keyTimesCount = keyTimes.size();
    381385    // Compare index + 1 to keyTimesCount because the last keyTimes entry is
    382386    // required to be 1, and percent can never exceed 1; i.e., the second last
    383387    // keyTimes entry defines the beginning of the final interval
    384388    for (index = 1; index + 1 < keyTimesCount; ++index) {
    385         if (m_keyTimes[index] > percent)
     389        if (keyTimes[index] > percent)
    386390            break;
    387391    }
    … …  
    402406float SVGAnimationElement::calculatePercentFromKeyPoints(float percent) const
    403407{
     408    const auto& keyTimes = this->keyTimes();
     409
    404410    ASSERT(!m_keyPoints.isEmpty());
    405411    ASSERT(calcMode() != CalcMode::Paced);
    406     ASSERT(m_keyTimes.size() > 1);
    407     ASSERT(m_keyPoints.size() == m_keyTimes.size());
     412    ASSERT(keyTimes.size() > 1);
     413    ASSERT(m_keyPoints.size() == keyTimes.size());
    408414
    409415    if (percent == 1)
    … …  
    411417
    412418    unsigned index = calculateKeyTimesIndex(percent);
    413     float fromPercent = m_keyTimes[index];
    414     float toPercent = m_keyTimes[index + 1];
     419    float fromPercent = keyTimes[index];
     420    float toPercent = keyTimes[index + 1];
    415421    float fromKeyPoint = m_keyPoints[index];
    416422    float toKeyPoint = m_keyPoints[index + 1];
    … …  
    430436float SVGAnimationElement::calculatePercentForFromTo(float percent) const
    431437{
    432     if (calcMode() == CalcMode::Discrete && m_keyTimes.size() == 2)
    433         return percent > m_keyTimes[1] ? 1 : 0;
     438    const auto& keyTimes = this->keyTimes();
     439    if (calcMode() == CalcMode::Discrete && keyTimes.size() == 2)
     440        return percent > keyTimes[1] ? 1 : 0;
    434441
    435442    return percent;
    … …  
    439446{
    440447    ASSERT(!m_keyPoints.isEmpty());
    441     ASSERT(m_keyPoints.size() == m_keyTimes.size());
     448    ASSERT(m_keyPoints.size() == keyTimes().size());
    442449    ASSERT(calcMode() != CalcMode::Paced);
    443450    effectivePercent = calculatePercentFromKeyPoints(percent);
    … …  
    469476        return currentValuesFromKeyPoints(percent, effectivePercent, from, to);
    470477   
    471     unsigned keyTimesCount = m_keyTimes.size();
     478    const auto& keyTimes = this->keyTimes();
     479    unsigned keyTimesCount = keyTimes.size();
    472480    ASSERT(!keyTimesCount || valuesCount == keyTimesCount);
    473     ASSERT(!keyTimesCount || (keyTimesCount > 1 && !m_keyTimes[0]));
     481    ASSERT(!keyTimesCount || (keyTimesCount > 1 && !keyTimes[0]));
    474482
    475483    unsigned index = calculateKeyTimesIndex(percent);
    … …  
    486494    float toPercent;
    487495    if (keyTimesCount) {
    488         fromPercent = m_keyTimes[index];
    489         toPercent = m_keyTimes[index + 1];
     496        fromPercent = keyTimes[index];
     497        toPercent = keyTimes[index + 1];
    490498    } else {       
    491499        index = static_cast<unsigned>(floorf(percent * (valuesCount - 1)));
    … …  
    514522        return;
    515523
     524    const auto& keyTimes = this->keyTimes();
     525
    516526    // These validations are appropriate for all animation modes.
    517     if (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) && m_keyPoints.size() != m_keyTimes.size())
     527    if (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) && m_keyPoints.size() != keyTimes.size())
    518528        return;
    519529
    … …  
    525535            || (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) && m_keyPoints.size() - 1 != splinesCount)
    526536            || (animationMode == AnimationMode::Values && m_values.size() - 1 != splinesCount)
    527             || (hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) && m_keyTimes.size() - 1 != splinesCount))
     537            || (hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) && keyTimes.size() - 1 != splinesCount))
    528538            return;
    529539    }
    … …  
    535545        return;
    536546    if ((animationMode == AnimationMode::FromTo || animationMode == AnimationMode::FromBy || animationMode == AnimationMode::To || animationMode == AnimationMode::By)
    537         && (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) && hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) && (m_keyTimes.size() < 2 || m_keyTimes.size() != m_keyPoints.size())))
     547        && (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) && hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) && (keyTimes.size() < 2 || keyTimes.size() != m_keyPoints.size())))
    538548        return;
    539549    if (animationMode == AnimationMode::FromTo)
    … …  
    549559    else if (animationMode == AnimationMode::Values) {
    550560        m_animationValid = m_values.size() >= 1
    551             && (calcMode == CalcMode::Paced || !hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) || hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (m_values.size() == m_keyTimes.size()))
    552             && (calcMode == CalcMode::Discrete || !m_keyTimes.size() || m_keyTimes.last() == 1)
     561            && (calcMode == CalcMode::Paced || !hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) || hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (m_values.size() == keyTimes.size()))
     562            && (calcMode == CalcMode::Discrete || !keyTimes.size() || keyTimes.last() == 1)
    553563            && (calcMode != CalcMode::Spline || ((m_keySplines.size() && (m_keySplines.size() == m_values.size() - 1)) || m_keySplines.size() == m_keyPoints.size() - 1))
    554             && (!hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (m_keyTimes.size() > 1 && m_keyTimes.size() == m_keyPoints.size()));
     564            && (!hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (keyTimes.size() > 1 && keyTimes.size() == m_keyPoints.size()));
    555565        if (m_animationValid)
    556566            m_animationValid = calculateToAtEndOfDurationValue(m_values.last());
    … …  
    558568            calculateKeyTimesForCalcModePaced();
    559569    } else if (animationMode == AnimationMode::Path)
    560         m_animationValid = calcMode == CalcMode::Paced || !hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (m_keyTimes.size() > 1 && m_keyTimes.size() == m_keyPoints.size());
     570        m_animationValid = calcMode == CalcMode::Paced || !hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (keyTimes.size() > 1 && keyTimes.size() == m_keyPoints.size());
    561571}
    562572
    … …  
    582592    } else if (!m_keyPoints.isEmpty() && calcMode != CalcMode::Paced)
    583593        effectivePercent = calculatePercentFromKeyPoints(percent);
    584     else if (m_keyPoints.isEmpty() && calcMode == CalcMode::Spline && m_keyTimes.size() > 1)
     594    else if (m_keyPoints.isEmpty() && calcMode == CalcMode::Spline && keyTimes().size() > 1)
    585595        effectivePercent = calculatePercentForSpline(percent, calculateKeyTimesIndex(percent));
    586596    else if (animationMode == AnimationMode::FromTo || animationMode == AnimationMode::To)

  • trunk/Source/WebCore/svg/SVGAnimationElement.h

    r251809 r275868  
    33 * Copyright (C) 2004, 2005, 2006 Rob Buis <buis@kde.org>
    44 * Copyright (C) 2007 Eric Seidel <eric@webkit.org>
    5  * Copyright (C) 2008-2019 Apple Inc. All rights reserved.
     5 * Copyright (C) 2008-2021 Apple Inc. All rights reserved.
    66 * Copyright (C) 2008 Cameron McCormack <cam@mcc.id.au>
    77 * Copyright (C) Research In Motion Limited 2011. All rights reserved.
    … …  
    124124    virtual Optional<float> calculateDistance(const String& /*fromString*/, const String& /*toString*/) = 0;
    125125
     126    const Vector<float>& keyTimes() const;
    126127    void currentValuesForValuesAnimation(float percent, float& effectivePercent, String& from, String& to);
    127128    void calculateKeyTimesForCalcModePaced();
    … …  
    138139    AttributeType m_attributeType { AttributeType::Auto };
    139140    Vector<String> m_values;
    140     Vector<float> m_keyTimes;
     141    Vector<float> m_keyTimesFromAttribute;
     142    Vector<float> m_keyTimesForPaced;
    141143    Vector<float> m_keyPoints;
    142144    Vector<UnitBezier> m_keySplines;
Note: See TracChangeset for help on using the changeset viewer.