Closed
Bug 492081
Opened 16 years ago
Closed 16 years ago
SVG Animation setCurrentTime fails (seeks to wrong time), when called after pausing & waiting
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(4 files)
590 bytes,
image/svg+xml
|
Details | |
591 bytes,
image/svg+xml
|
Details | |
1010 bytes,
patch
|
Details | Diff | Splinter Review | |
4.05 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
Currently, setCurrentTime ends up seeking to the wrong time, if it's called significantly after "pauseAnimations" was called.
In fact, if "n" seconds have elapsed since pauseAnimations was called, then setCurrentTime will be off by about "n" seconds.
STEPS TO REPRODUCE:
1. Enable the "svg.smil.enabled" pref, in a mozilla-central nightly
2. Load testcase
EXPECTED RESULTS:
"Success" alert should pop up (indicating that getCurrentTime gave us back the value that we set with setCurrentTime)
ACTUAL RESULTS:
"Failure" alert pops up (indicating that getCurrentTime gave us back an unexpected value)
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090507 Minefield/3.6a1pre
Opera v9.64 shows expected results.
Assignee | ||
Updated•16 years ago
|
Summary: SVG Animation setCurrentTime fails (sets wrong time), when called after pausing & waiting → SVG Animation setCurrentTime fails (seeks to wrong time), when called after pausing & waiting
Assignee | ||
Comment 1•16 years ago
|
||
Assignee | ||
Comment 2•16 years ago
|
||
(In reply to comment #0)
> In fact, if "n" seconds have elapsed since pauseAnimations was called, then
> setCurrentTime will be off by about "n" seconds.
This testcase demonstrates this aspect -- it waits for 1 sec before calling setCurrentTime, and the result is that we're off by ~1 sec.
Assignee | ||
Updated•16 years ago
|
Attachment #376447 -
Attachment description: testcase 1 → testcase 1: wait 200 ms
Assignee | ||
Comment 3•16 years ago
|
||
I think this fixes it. Basically, the situation with current mozilla-central is this:
* mParentOffset is effectively the unix time of our document start time.
* When we're paused, GetCurrentTime effectively returns mPauseStart - mParentOffset (which is cached as mCurrentTime in UpdateCurrentTime())
* In setCurrentTime(), we adjust mParentOffset (using an updated value of PR_Now() via GetParentTime()), but we **don't** update mPauseStart
As a result, if we wait longer to call SetCurrentTime, then mParentOffset gets larger (whereas mPauseStart doesn't change), and so our effective "current time" shrinks.
This patch simply makes us update mPauseStart with mParentOffset in SetCurrentTime, keeping it exactly "aSeekTo" milliseconds beyond mParentOffset.
I still need to run this through reftests & mochitests, but I'm pretty sure this is correct.
Assignee | ||
Updated•16 years ago
|
Attachment #376449 -
Attachment description: testcase 1: wait 1 sec → testcase 2: wait 1 sec
Assignee | ||
Comment 4•16 years ago
|
||
Yup, this passes the SMIL mochitests & reftests.
Here's the same fix, with a mochitest attached. I've confirmed that the mochitest fails[1] pre-patching and passes post-patching.
[1] By "fails", I mean it fails on all the checks after setTimeout calls, *except* for the one check with setCurrentTime(10000000), as that value is apparently large enough to crop the (relatively small) accumulated wait time from its float representation.
Attachment #376480 -
Flags: superreview?(roc)
Attachment #376480 -
Flags: review?(roc)
Attachment #376480 -
Flags: superreview?(roc)
Attachment #376480 -
Flags: superreview+
Attachment #376480 -
Flags: review?(roc)
Attachment #376480 -
Flags: review+
Assignee | ||
Comment 5•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•