Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[SVG] Interval should not be created if a value in begin-value-list d…
…oesn't have a matching value in end-value-list

https://bugs.webkit.org/show_bug.cgi?id=257105

Reviewed by Simon Fraser.

This patch aligns WebKit with Gecko / Firefox and Blink / Chromium.

Merge: https://src.chromium.org/viewvc/blink?view=revision&revision=157739

Here http://www.w3.org/TR/SMIL3/smil-timing.html#q90 , it says
that if all ends are before the current begin, then it is a bad interval
and current begin will be discarded.
Existing code is treating it as a valid interval with end as indefinite.
Modified the logic to be in line with the pseudo code.

* Source/WebCore/svg/animation/SVGSMILElement.cpp:
(SVGSMILElement::findInstanceTime): change 'return' value
(SVGSMILElement::resolveNextInterval): Change to 'bool'
(SVGSMILElement::seekToIntervalCorrespondingToTime): Modify 'resolveNextInterval' in 'if' condition
* Source/WebCore/svg/animation/SVGSMILElement.h: 'resolveNextInterval' to bool
* LayoutTests/resources/SVGAnimationTestCase-testharness.js: Move this to 'resource' folder as well
* LayoutTests/svg/animations/animate-no-matching-end-value.html: Add Test Case
* LayoutTests/svg/animations/animate-no-matching-end-value-expected.txt: Add Test Case Expectation

Canonical link: https://commits.webkit.org/264635@main
  • Loading branch information
Ahmad-S792 authored and Ahmad Saleem committed May 27, 2023
1 parent 76418fb commit 0d691ef
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 5 deletions.
102 changes: 102 additions & 0 deletions LayoutTests/resources/SVGAnimationTestCase-testharness.js
@@ -0,0 +1,102 @@
// NOTE(edvardt):
// This file is a slimmed down wrapper for the old SVGAnimationTestCase.js,
// it has some convenience functions and should not be used for new tests.
// New tests should not build on this API as it's just meant to keep things
// working.

// Helper functions
const xlinkNS = "http://www.w3.org/1999/xlink"

function expectFillColor(element, red, green, blue, message) {
let color = window.getComputedStyle(element, null).fill;
var re = new RegExp("rgba?\\(([^, ]*), ([^, ]*), ([^, ]*)(?:, )?([^, ]*)\\)");
rgb = re.exec(color);

assert_approx_equals(Number(rgb[1]), red, 2.0, message);
assert_approx_equals(Number(rgb[2]), green, 2.0, message);
assert_approx_equals(Number(rgb[3]), blue, 2.0, message);
}

function expectColor(element, red, green, blue, property) {
if (typeof property != "string")
color = getComputedStyle(element).getPropertyValue("color");
else
color = getComputedStyle(element).getPropertyValue(property);
var re = new RegExp("rgba?\\(([^, ]*), ([^, ]*), ([^, ]*)(?:, )?([^, ]*)\\)");
rgb = re.exec(color);
assert_approx_equals(Number(rgb[1]), red, 2.0);
assert_approx_equals(Number(rgb[2]), green, 2.0);
assert_approx_equals(Number(rgb[3]), blue, 2.0);
}

function createSVGElement(type) {
return document.createElementNS("http://www.w3.org/2000/svg", type);
}

// Inspired by Layoutests/animations/animation-test-helpers.js
function moveAnimationTimelineAndSample(index) {
var animationId = expectedResults[index][0];
var time = expectedResults[index][1];
var sampleCallback = expectedResults[index][2];
var animation = rootSVGElement.ownerDocument.getElementById(animationId);

// If we want to sample the animation end, add a small delta, to reliable point past the end of the animation.
newTime = time;

// The sample time is relative to the start time of the animation, take that into account.
rootSVGElement.setCurrentTime(newTime);

// NOTE(edvardt):
// This is a dumb hack, some of the old tests sampled before the animation start, this
// isn't technically part of the animation tests and is "impossible" to translate since
// tests start automatically. Thus I solved it by skipping it.
if (time != 0.0)
sampleCallback();
}

var currentTest = 0;
var expectedResults;

function sampleAnimation(t) {
if (currentTest == expectedResults.length) {
t.done();
return;
}

moveAnimationTimelineAndSample(currentTest);
++currentTest;

step_timeout(t.step_func(function () { sampleAnimation(t); }), 0);
}

function runAnimationTest(t, expected) {
if (!expected)
throw("Expected results are missing!");
if (currentTest > 0)
throw("Not allowed to call runAnimationTest() twice");

expectedResults = expected;
testCount = expectedResults.length;
currentTest = 0;

step_timeout(t.step_func(function () { sampleAnimation(this); }), 50);
}

function smil_async_test(func) {
async_test(t => {
window.onload = t.step_func(function () {
// Pause animations, we'll drive them manually.
// This also ensures that the timeline is paused before
// it starts. This should make the instance time of the below
// 'click' (for instance) 0, and hence minimize rounding
// errors for the addition in moveAnimationTimelineAndSample.
rootSVGElement.pauseAnimations();

// If eg. an animation is running with begin="0s", and
// we want to sample the first time, before the animation
// starts, then we can't delay the testing by using an
// onclick event, as the animation would be past start time.
func(t);
});
});
}
@@ -0,0 +1,3 @@

PASS Test animate intervals having begin-value without a matching end-value

42 changes: 42 additions & 0 deletions LayoutTests/svg/animations/animate-no-matching-end-value.html
@@ -0,0 +1,42 @@
<!DOCTYPE html>
<title>Test animate intervals having begin-value without a matching end-value</title>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../../resources/SVGAnimationTestCase-testharness.js"></script>
<svg>
<!-- No interval will be created at 5 s since the end attribute does
not contain a rule to end an interval starting later than 4 s. -->
<rect x="0" y="0" width="50" height="50" fill="green">
<animate id="anim" attributeName="visibility" to="visible" begin="0s" dur="6s" />
<animate attributeName="x" from="100" to="100" begin="1;3;5" end="2;4" />
</rect>
</svg>
<script>
// Setup animation test
var rootSVGElement = document.querySelector("svg");
var rect1 = rootSVGElement.ownerDocument.getElementsByTagName("rect")[0];
var epsilon = 0.15;
function sample1() {
assert_equals(rect1.x.animVal.value, 0);
assert_equals(rect1.x.baseVal.value, 0);
}

function sample2() {
assert_equals(rect1.x.animVal.value, 100);
assert_equals(rect1.x.baseVal.value, 0);
}

smil_async_test(t => {
const expectedValues = [
// [animationId, time, sampleCallback]
["anim", 0.1, sample1],
["anim", 1.1, sample2],
["anim", 2.1, sample1],
["anim", 3.1, sample2],
["anim", 4.1, sample1],
["anim", 5.1, sample1]
];

runAnimationTest(t, expectedValues);
});
</script>
12 changes: 8 additions & 4 deletions Source/WebCore/svg/animation/SVGSMILElement.cpp
Expand Up @@ -788,7 +788,7 @@ SMILTime SVGSMILElement::findInstanceTime(BeginOrEnd beginOrEnd, SMILTime minimu
return SMILTime::unresolved();

if (currentTime < minimumTime)
return beginOrEnd == Begin ? SMILTime::unresolved() : SMILTime::indefinite();
return SMILTime::unresolved();
if (currentTime > minimumTime)
return currentTime;

Expand Down Expand Up @@ -899,7 +899,7 @@ void SVGSMILElement::resolveFirstInterval()
}
}

void SVGSMILElement::resolveNextInterval()
bool SVGSMILElement::resolveNextInterval()
{
SMILTime begin;
SMILTime end;
Expand All @@ -911,7 +911,9 @@ void SVGSMILElement::resolveNextInterval()
m_intervalEnd = end;
notifyDependentsIntervalChanged(NewInterval);
m_nextProgressTime = std::min(m_nextProgressTime, m_intervalBegin);
return true;
}
return false;
}

SMILTime SVGSMILElement::nextProgressTime() const
Expand Down Expand Up @@ -1013,13 +1015,15 @@ void SVGSMILElement::seekToIntervalCorrespondingToTime(SMILTime elapsed)
if (nextBegin < m_intervalEnd && elapsed >= nextBegin) {
// End current interval, and start a new interval from the 'nextBegin' time.
m_intervalEnd = nextBegin;
resolveNextInterval();
if (!resolveNextInterval())
break;
continue;
}

// If the desired 'elapsed' time is past the current interval, advance to the next.
if (elapsed >= m_intervalEnd) {
resolveNextInterval();
if (!resolveNextInterval())
break;
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/svg/animation/SVGSMILElement.h
Expand Up @@ -137,7 +137,7 @@ class SVGSMILElement : public SVGElement {
enum BeginOrEnd { Begin, End };
SMILTime findInstanceTime(BeginOrEnd, SMILTime minimumTime, bool equalsMinimumOK) const;
void resolveFirstInterval();
void resolveNextInterval();
bool resolveNextInterval();
void resolveInterval(bool first, SMILTime& beginResult, SMILTime& endResult) const;
SMILTime resolveActiveEnd(SMILTime resolvedBegin, SMILTime resolvedEnd) const;
SMILTime repeatingDuration() const;
Expand Down

0 comments on commit 0d691ef

Please sign in to comment.