RESOLVED FIXED 85372
Fix multiple begin values support - especially with seeking through setCurrentTime
https://bugs.webkit.org/show_bug.cgi?id=85372
Summary Fix multiple begin values support - especially with seeking through setCurren...
Nikolas Zimmermann
Reported 2012-05-02 06:44:16 PDT
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.
Attachments
Patch (27.37 KB, patch)
2012-05-02 06:49 PDT, Nikolas Zimmermann
zherczeg: review+
Nikolas Zimmermann
Comment 1 2012-05-02 06:49:04 PDT
WebKit Review Bot
Comment 2 2012-05-02 06:52:28 PDT
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
Comment 3 2012-05-02 08:32:42 PDT
(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
Comment 4 2012-05-03 00:33:46 PDT
(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 5 2012-05-03 01:24:46 PDT
Comment on attachment 139804 [details] Patch r=me. Fix the complain of the style bot.
Nikolas Zimmermann
Comment 6 2012-05-03 01:44:49 PDT
Note You need to log in before you can comment on or make changes to this bug.