Bug 85372 – Fix multiple begin values support - especially with seeking through setCurrentTime

• Component: SVG
• Status: RESOLVED FIXED

[Nikolas Zimmermann]

Multiple begin values aka. begin="0s; 2s" aren't correctly handled - resulting in broken & unexpected behavior.
Supporting seeking properly on documents containing such animations is very important, otherwise we can't reliable test animations using either reftests or the SVG JS animation test framework.

Testcase:
<rect height="100" fill="green">
    <animate attributeName="width" begin="0s; 2s" dur="8s" from="0" to="100" fill="freeze"/>
</rect>

What's expected?
Two times should be contained in the 'begin' times list in SVGSMILElement: m_beginTimes = { 0s, 2s }.
The initial first resolved interval is: m_intervalBegin=0.0s, m_intervalEnd=8.0s.

During t=0s..1.9999s the m_intervalBegin/m_intervalEnd are correct.
At t=2s, a new interval can be started. m_intervalEnd should be set to nextBeginTime, where nextBeginTime=2s.
The current interval should get cropped to: m_intervalBegin=0s, m_intervalEnd=2s. The following call to resolveNextInterval() sees that elapsed >= m_intervalEnd, and thus moves on to the next interval.
m_intervalBegin should be 2s and m_intervalEnd=10s after that.

In trunk this behavior is only partly implemented and broken. Especially broken together with seeking via SVGSVGElement.setCurrentTime.
That's because we don't correctly seek to the right interval in case of multiple begin values, eg. if we sample an animation with begin="0s; 3s" dur="6s" we always remain in the first interval and don't move on.

[Nikolas Zimmermann]

Created attachment 139804 [details]
Patch

[WebKit Review Bot]

Attachment 139804 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/svg/..." exit_code: 1
Source/WebCore/svg/animation/SVGSMILElement.cpp:786:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nikolas Zimmermann]

(In reply to comment #2)
> Attachment 139804 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/svg/..." exit_code: 1
> Source/WebCore/svg/animation/SVGSMILElement.cpp:786:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
This error is in purpose. I can't use !smilTime, I have to use smilTime == 0

[Nikolas Zimmermann]

(In reply to comment #3)
> > Source/WebCore/svg/animation/SVGSMILElement.cpp:786:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
> This error is in purpose. I can't use !smilTime, I have to use smilTime == 0

Another way to get rid of the warning is to use !smilTime.value(), instead of smilTime == 0.
I could include this upon landing, if desired.

[Zoltan Herczeg]

Comment on attachment 139804 [details]
Patch

r=me. Fix the complain of the style bot.

[Nikolas Zimmermann]

Committed r115947: <http://trac.webkit.org/changeset/115947>