Last Comment Bug 733744 - animateTransform with a very small duration (less than 1 millisecond) no longer animates
: animateTransform with a very small duration (less than 1 millisecond) no long...
Status: REOPENED
[see comment 11 for explanation]
:
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
http://hoffmann.bplaced.net/svgtest/t...
Depends on:
Blocks: 651036
  Show dependency treegraph
 
Reported: 2012-03-07 06:19 PST by Robert Longson
Modified: 2013-05-20 18:35 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase 1 (no red should be visible) (483 bytes, image/svg+xml)
2012-03-07 14:03 PST, Daniel Holbert [:dholbert]
no flags Details
reference case 1 (482 bytes, image/svg+xml)
2012-03-07 14:04 PST, Daniel Holbert [:dholbert]
no flags Details

Description Robert Longson 2012-03-07 06:19:52 PST
Apparently this example used to work in Firefox 4.
Comment 1 Daniel Holbert [:dholbert] 2012-03-07 13:32:27 PST
(In reply to Robert Longson from comment #0)
> Apparently this example used to work in Firefox 4.

Confirmed -- in Firefox 4, the parallelogram snaps to a rounded square after 3 seconds.
Comment 2 Daniel Holbert [:dholbert] 2012-03-07 13:38:55 PST
This apparently depends on the very small dur="0.0001s" attribute.

If I increase that to 0.1, then the animation succeeds.
Comment 3 Daniel Holbert [:dholbert] 2012-03-07 13:41:34 PST
Last good nightly: 2011-04-19
First bad nightly: 2011-04-20

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7d4eb3d3c9de&tochange=45b20f137549
which points to bug 651036
Comment 4 Daniel Holbert [:dholbert] 2012-03-07 14:03:21 PST
Created attachment 603850 [details]
testcase 1 (no red should be visible)
Comment 5 Daniel Holbert [:dholbert] 2012-03-07 14:04:35 PST
Created attachment 603851 [details]
reference case 1

Here's a reference case, which exactly matches the just-attached testcase except that the animation's duration is marginally longer (0.001s instead of 0.0001s).
Comment 6 Daniel Holbert [:dholbert] 2012-03-07 14:05:59 PST
(That's all the poking I'm going to do on this for now -- if someone else wants to take this and figure out what's going wrong, please be my guest! :))
Comment 7 Brian Birtles (:birtles) 2012-03-07 16:11:26 PST
This is a known issue with our implementation of frozen to-animation. The way frozen to-animation is defined is really counter-intuitive. See:

  http://www.w3.org/Graphics/SVG/WG/wiki/F2F/Auckland_2011/Animation_improvements#Issue:_Frozen_to-animation_is_broken

To support this properly, you need to set a milestone when the frozen animation ends, and actually sample the animation value then. Currently we set milestones, by we only sample the timing model at these times, not the animation values.

For frozen to-animations we currently just use the last sampled value before the animation got frozen. So if the sample rate is not fast enough, or you do a sync, you'll get the wrong result. Hence this bug.

I'd really like to change this behaviour. It's not just that it's hard to implement, but counter-intuitive.

I proposed this on www-svg:

  http://lists.w3.org/Archives/Public/www-svg/2011Mar/0082.html

But I didn't have the stamina to see it through.

I think we should just align with Opera here as I think it does what authors expect.
Comment 8 Robert Longson 2012-03-11 03:46:19 PDT
So this is a duplicate of bug 658189 which also has animations with a small duration.
Comment 9 Robert Longson 2012-03-11 03:46:53 PDT
Feel free to undup if I got this wrong.

*** This bug has been marked as a duplicate of bug 658189 ***
Comment 10 Brian Birtles (:birtles) 2012-03-11 16:11:34 PDT
(In reply to Robert Longson from comment #9)
> Feel free to undup if I got this wrong.

Yeah, the symptoms are very similar. However, bug 658189 doesn't have any frozen to-animations so it doesn't appear to be the same root cause. It's a shame because I'm still not sure what the cause is behind bug 658189.

(There's still a remote chance this is a dupe, if, for example, the <set> animations in that bug are using some of the frozen to-animation code but from memory that's not the case.)
Comment 11 Brian Birtles (:birtles) 2012-05-15 01:04:08 PDT
I've looked into this and actually what I think is happening is that, since we store times as integers representing milliseconds, dur="0.0001s" is represented as 0 and dur="0.001s" is represented as 1.

Now, SMIL says that the duration must be non-zero. If it is zero it is an error, and if there is an error, we should behave as if the duration was indefinite.

Under the previous discrete to-animation behaviour, where we never visited the underlying value, an indefinite duration would still end up showing the to-value. With the behaviour implemented in bug 651036--where we visit both the underlying value and the to-value--we'll just get stuck on the first value.

I'm not really sure what we ought to do here. It comes down to the precision used for representing times.

I still want to change what we do for frozen to-animation (I have a patch for it) but that's a separate bug.
Comment 12 Robert Longson 2012-05-15 03:33:59 PDT
We could store times as doubles perhaps?
Comment 13 Brian Birtles (:birtles) 2012-05-15 18:47:11 PDT
(In reply to Robert Longson from comment #12)
> We could store times as doubles perhaps?

Yeah, I've been trying to avoid that since it's a big change that might introduce bugs (or more likely, test regressions) due to floating-point errors. I'm not sure if this bug warrants the change.

An alternative is to store times as integer number of microseconds which would still give us a range of +/- 292,263 years.
Comment 14 Brian Birtles (:birtles) 2012-05-15 18:54:52 PDT
Filed bug 755603 for fixing the frozen to-animation behaviour.

Note You need to log in before you can comment on or make changes to this bug.