Diff
Modified: trunk/LayoutTests/ChangeLog (275867 => 275868)
--- trunk/LayoutTests/ChangeLog 2021-04-13 08:04:07 UTC (rev 275867)
+++ trunk/LayoutTests/ChangeLog 2021-04-13 08:05:24 UTC (rev 275868)
@@ -1,3 +1,15 @@
+2021-04-13 Said Abou-Hallawa <s...@apple.com>
+
+ SVG paced value animations overwrite user-provided keyTimes
+ https://bugs.webkit.org/show_bug.cgi?id=109010
+
+ Reviewed by Ryosuke Niwa.
+
+ 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.
+
2021-04-12 Xabier Rodriguez Calvar <calva...@igalia.com>
[GTK] media/track/text-track-cue-is-reachable.html is flaky crashing in X11 release
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (275867 => 275868)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2021-04-13 08:04:07 UTC (rev 275867)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2021-04-13 08:05:24 UTC (rev 275868)
@@ -1,3 +1,12 @@
+2021-04-13 Said Abou-Hallawa <s...@apple.com>
+
+ SVG paced value animations overwrite user-provided keyTimes
+ https://bugs.webkit.org/show_bug.cgi?id=109010
+
+ Reviewed by Ryosuke Niwa.
+
+ * web-platform-tests/svg/animations/scripted/paced-value-animation-overwrites-keyTimes-expected.txt:
+
2021-04-12 Commit Queue <commit-qu...@webkit.org>
Unreviewed, reverting r275793.
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/svg/animations/scripted/paced-value-animation-overwrites-keyTimes-expected.txt (275867 => 275868)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/svg/animations/scripted/paced-value-animation-overwrites-keyTimes-expected.txt 2021-04-13 08:04:07 UTC (rev 275867)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/svg/animations/scripted/paced-value-animation-overwrites-keyTimes-expected.txt 2021-04-13 08:05:24 UTC (rev 275868)
@@ -1,3 +1,3 @@
-FAIL Paced value animation doesn't overwrite keyTimes assert_approx_equals: expected 150 +/- 5 but got 120
+PASS Paced value animation doesn't overwrite keyTimes
Added: trunk/LayoutTests/svg/animations/animate-calcMode-paced-overwrite-key-times-expected.html (0 => 275868)
--- trunk/LayoutTests/svg/animations/animate-calcMode-paced-overwrite-key-times-expected.html (rev 0)
+++ trunk/LayoutTests/svg/animations/animate-calcMode-paced-overwrite-key-times-expected.html 2021-04-13 08:05:24 UTC (rev 275868)
@@ -0,0 +1,6 @@
+<body>
+ <svg id="svg" width="500" height="500">
+ <rect x="150" y="0" width="100" height="100" fill="green"/>
+ </svg>
+ </script>
+</body>
Added: trunk/LayoutTests/svg/animations/animate-calcMode-paced-overwrite-key-times.html (0 => 275868)
--- trunk/LayoutTests/svg/animations/animate-calcMode-paced-overwrite-key-times.html (rev 0)
+++ trunk/LayoutTests/svg/animations/animate-calcMode-paced-overwrite-key-times.html 2021-04-13 08:05:24 UTC (rev 275868)
@@ -0,0 +1,26 @@
+<body>
+ <svg id="svg" width="500" height="500">
+ <rect x="150" y="0" width="100" height="100" fill="red"/>
+ <rect width="100" height="100" fill="green">
+ <animate id="animate1" attributeName="x" dur="10s" calcMode="paced" values="100; 150; 200;" keyTimes="0; 0.2; 1"/>
+ </rect>
+ </svg>
+ <script>
+ window.addEventListener('load', (event) => {
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ window.requestAnimationFrame(() => {
+ let svg = document.getElementById('svg');
+ let animate1 = document.getElementById('animate1');
+
+ animate1.setAttribute('calcMode', 'linear');
+ svg.pauseAnimations();
+ svg.setCurrentTime(2);
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+ });
+ });
+ </script>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (275867 => 275868)
--- trunk/Source/WebCore/ChangeLog 2021-04-13 08:04:07 UTC (rev 275867)
+++ trunk/Source/WebCore/ChangeLog 2021-04-13 08:05:24 UTC (rev 275868)
@@ -1,3 +1,37 @@
+2021-04-13 Said Abou-Hallawa <s...@apple.com>
+
+ SVG paced value animations overwrite user-provided keyTimes
+ https://bugs.webkit.org/show_bug.cgi?id=109010
+
+ Reviewed by Ryosuke Niwa.
+
+ 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:
+
2021-04-12 Ryosuke Niwa <rn...@webkit.org>
TextManipulationController should use weak pointers to Node
Modified: trunk/Source/WebCore/svg/SVGAnimationElement.cpp (275867 => 275868)
--- trunk/Source/WebCore/svg/SVGAnimationElement.cpp 2021-04-13 08:04:07 UTC (rev 275867)
+++ trunk/Source/WebCore/svg/SVGAnimationElement.cpp 2021-04-13 08:05:24 UTC (rev 275868)
@@ -2,7 +2,7 @@
* Copyright (C) 2004, 2005 Nikolas Zimmermann <zimmerm...@kde.org>
* Copyright (C) 2004, 2005, 2006, 2007 Rob Buis <b...@kde.org>
* Copyright (C) 2007 Eric Seidel <e...@webkit.org>
- * Copyright (C) 2008, 2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2021 Apple Inc. All rights reserved.
* Copyright (C) 2009 Cameron McCormack <c...@mcc.id.au>
* Copyright (C) Research In Motion Limited 2010. All rights reserved.
* Copyright (C) 2014 Adobe Systems Incorporated. All rights reserved.
@@ -161,7 +161,7 @@
}
if (name == SVGNames::keyTimesAttr) {
- parseKeyTimes(value, m_keyTimes, true);
+ parseKeyTimes(value, m_keyTimesFromAttribute, true);
return;
}
@@ -346,8 +346,7 @@
if (valuesCount == 1)
return;
- // FIXME, webkit.org/b/109010: m_keyTimes should not be modified in this function.
- m_keyTimes.clear();
+ m_keyTimesForPaced.clear();
Vector<float> keyTimesForPaced;
float totalDistance = 0;
@@ -368,21 +367,26 @@
keyTimesForPaced[n] = keyTimesForPaced[n - 1] + keyTimesForPaced[n] / totalDistance;
keyTimesForPaced[keyTimesForPaced.size() - 1] = 1;
- // Use key times calculated based on pacing instead of the user provided ones.
- m_keyTimes = keyTimesForPaced;
+ m_keyTimesForPaced = WTFMove(keyTimesForPaced);
}
static inline double solveEpsilon(double duration) { return 1 / (200 * duration); }
+const Vector<float>& SVGAnimationElement::keyTimes() const
+{
+ return calcMode() == CalcMode::Paced ? m_keyTimesForPaced : m_keyTimesFromAttribute;
+}
+
unsigned SVGAnimationElement::calculateKeyTimesIndex(float percent) const
{
+ const auto& keyTimes = this->keyTimes();
unsigned index;
- unsigned keyTimesCount = m_keyTimes.size();
+ unsigned keyTimesCount = keyTimes.size();
// Compare index + 1 to keyTimesCount because the last keyTimes entry is
// required to be 1, and percent can never exceed 1; i.e., the second last
// keyTimes entry defines the beginning of the final interval
for (index = 1; index + 1 < keyTimesCount; ++index) {
- if (m_keyTimes[index] > percent)
+ if (keyTimes[index] > percent)
break;
}
return --index;
@@ -401,17 +405,19 @@
float SVGAnimationElement::calculatePercentFromKeyPoints(float percent) const
{
+ const auto& keyTimes = this->keyTimes();
+
ASSERT(!m_keyPoints.isEmpty());
ASSERT(calcMode() != CalcMode::Paced);
- ASSERT(m_keyTimes.size() > 1);
- ASSERT(m_keyPoints.size() == m_keyTimes.size());
+ ASSERT(keyTimes.size() > 1);
+ ASSERT(m_keyPoints.size() == keyTimes.size());
if (percent == 1)
return m_keyPoints[m_keyPoints.size() - 1];
unsigned index = calculateKeyTimesIndex(percent);
- float fromPercent = m_keyTimes[index];
- float toPercent = m_keyTimes[index + 1];
+ float fromPercent = keyTimes[index];
+ float toPercent = keyTimes[index + 1];
float fromKeyPoint = m_keyPoints[index];
float toKeyPoint = m_keyPoints[index + 1];
@@ -429,8 +435,9 @@
float SVGAnimationElement::calculatePercentForFromTo(float percent) const
{
- if (calcMode() == CalcMode::Discrete && m_keyTimes.size() == 2)
- return percent > m_keyTimes[1] ? 1 : 0;
+ const auto& keyTimes = this->keyTimes();
+ if (calcMode() == CalcMode::Discrete && keyTimes.size() == 2)
+ return percent > keyTimes[1] ? 1 : 0;
return percent;
}
@@ -438,7 +445,7 @@
void SVGAnimationElement::currentValuesFromKeyPoints(float percent, float& effectivePercent, String& from, String& to) const
{
ASSERT(!m_keyPoints.isEmpty());
- ASSERT(m_keyPoints.size() == m_keyTimes.size());
+ ASSERT(m_keyPoints.size() == keyTimes().size());
ASSERT(calcMode() != CalcMode::Paced);
effectivePercent = calculatePercentFromKeyPoints(percent);
unsigned index = effectivePercent == 1 ? m_values.size() - 2 : static_cast<unsigned>(effectivePercent * (m_values.size() - 1));
@@ -468,9 +475,10 @@
if (!m_keyPoints.isEmpty() && calcMode != CalcMode::Paced)
return currentValuesFromKeyPoints(percent, effectivePercent, from, to);
- unsigned keyTimesCount = m_keyTimes.size();
+ const auto& keyTimes = this->keyTimes();
+ unsigned keyTimesCount = keyTimes.size();
ASSERT(!keyTimesCount || valuesCount == keyTimesCount);
- ASSERT(!keyTimesCount || (keyTimesCount > 1 && !m_keyTimes[0]));
+ ASSERT(!keyTimesCount || (keyTimesCount > 1 && !keyTimes[0]));
unsigned index = calculateKeyTimesIndex(percent);
if (calcMode == CalcMode::Discrete) {
@@ -485,8 +493,8 @@
float fromPercent;
float toPercent;
if (keyTimesCount) {
- fromPercent = m_keyTimes[index];
- toPercent = m_keyTimes[index + 1];
+ fromPercent = keyTimes[index];
+ toPercent = keyTimes[index + 1];
} else {
index = static_cast<unsigned>(floorf(percent * (valuesCount - 1)));
fromPercent = static_cast<float>(index) / (valuesCount - 1);
@@ -513,8 +521,10 @@
if (!hasValidAttributeType())
return;
+ const auto& keyTimes = this->keyTimes();
+
// These validations are appropriate for all animation modes.
- if (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) && m_keyPoints.size() != m_keyTimes.size())
+ if (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) && m_keyPoints.size() != keyTimes.size())
return;
AnimationMode animationMode = this->animationMode();
@@ -524,7 +534,7 @@
if (!splinesCount
|| (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) && m_keyPoints.size() - 1 != splinesCount)
|| (animationMode == AnimationMode::Values && m_values.size() - 1 != splinesCount)
- || (hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) && m_keyTimes.size() - 1 != splinesCount))
+ || (hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) && keyTimes.size() - 1 != splinesCount))
return;
}
@@ -534,7 +544,7 @@
if (animationMode == AnimationMode::None)
return;
if ((animationMode == AnimationMode::FromTo || animationMode == AnimationMode::FromBy || animationMode == AnimationMode::To || animationMode == AnimationMode::By)
- && (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) && hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) && (m_keyTimes.size() < 2 || m_keyTimes.size() != m_keyPoints.size())))
+ && (hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) && hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) && (keyTimes.size() < 2 || keyTimes.size() != m_keyPoints.size())))
return;
if (animationMode == AnimationMode::FromTo)
m_animationValid = calculateFromAndToValues(from, to);
@@ -548,16 +558,16 @@
m_animationValid = calculateFromAndByValues(emptyString(), by);
else if (animationMode == AnimationMode::Values) {
m_animationValid = m_values.size() >= 1
- && (calcMode == CalcMode::Paced || !hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) || hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (m_values.size() == m_keyTimes.size()))
- && (calcMode == CalcMode::Discrete || !m_keyTimes.size() || m_keyTimes.last() == 1)
+ && (calcMode == CalcMode::Paced || !hasAttributeWithoutSynchronization(SVGNames::keyTimesAttr) || hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (m_values.size() == keyTimes.size()))
+ && (calcMode == CalcMode::Discrete || !keyTimes.size() || keyTimes.last() == 1)
&& (calcMode != CalcMode::Spline || ((m_keySplines.size() && (m_keySplines.size() == m_values.size() - 1)) || m_keySplines.size() == m_keyPoints.size() - 1))
- && (!hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (m_keyTimes.size() > 1 && m_keyTimes.size() == m_keyPoints.size()));
+ && (!hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (keyTimes.size() > 1 && keyTimes.size() == m_keyPoints.size()));
if (m_animationValid)
m_animationValid = calculateToAtEndOfDurationValue(m_values.last());
if (calcMode == CalcMode::Paced && m_animationValid)
calculateKeyTimesForCalcModePaced();
} else if (animationMode == AnimationMode::Path)
- m_animationValid = calcMode == CalcMode::Paced || !hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (m_keyTimes.size() > 1 && m_keyTimes.size() == m_keyPoints.size());
+ m_animationValid = calcMode == CalcMode::Paced || !hasAttributeWithoutSynchronization(SVGNames::keyPointsAttr) || (keyTimes.size() > 1 && keyTimes.size() == m_keyPoints.size());
}
void SVGAnimationElement::updateAnimation(float percent, unsigned repeatCount)
@@ -581,7 +591,7 @@
}
} else if (!m_keyPoints.isEmpty() && calcMode != CalcMode::Paced)
effectivePercent = calculatePercentFromKeyPoints(percent);
- else if (m_keyPoints.isEmpty() && calcMode == CalcMode::Spline && m_keyTimes.size() > 1)
+ else if (m_keyPoints.isEmpty() && calcMode == CalcMode::Spline && keyTimes().size() > 1)
effectivePercent = calculatePercentForSpline(percent, calculateKeyTimesIndex(percent));
else if (animationMode == AnimationMode::FromTo || animationMode == AnimationMode::To)
effectivePercent = calculatePercentForFromTo(percent);
Modified: trunk/Source/WebCore/svg/SVGAnimationElement.h (275867 => 275868)
--- trunk/Source/WebCore/svg/SVGAnimationElement.h 2021-04-13 08:04:07 UTC (rev 275867)
+++ trunk/Source/WebCore/svg/SVGAnimationElement.h 2021-04-13 08:05:24 UTC (rev 275868)
@@ -2,7 +2,7 @@
* Copyright (C) 2004, 2005 Nikolas Zimmermann <zimmerm...@kde.org>
* Copyright (C) 2004, 2005, 2006 Rob Buis <b...@kde.org>
* Copyright (C) 2007 Eric Seidel <e...@webkit.org>
- * Copyright (C) 2008-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2021 Apple Inc. All rights reserved.
* Copyright (C) 2008 Cameron McCormack <c...@mcc.id.au>
* Copyright (C) Research In Motion Limited 2011. All rights reserved.
*
@@ -123,6 +123,7 @@
virtual void calculateAnimatedValue(float percent, unsigned repeatCount) = 0;
virtual Optional<float> calculateDistance(const String& /*fromString*/, const String& /*toString*/) = 0;
+ const Vector<float>& keyTimes() const;
void currentValuesForValuesAnimation(float percent, float& effectivePercent, String& from, String& to);
void calculateKeyTimesForCalcModePaced();
float calculatePercentFromKeyPoints(float percent) const;
@@ -137,7 +138,8 @@
AttributeType m_attributeType { AttributeType::Auto };
Vector<String> m_values;
- Vector<float> m_keyTimes;
+ Vector<float> m_keyTimesFromAttribute;
+ Vector<float> m_keyTimesForPaced;
Vector<float> m_keyPoints;
Vector<UnitBezier> m_keySplines;
String m_lastValuesAnimationFrom;
