733744 - animateTransform with a very small duration (less than 1 millisecond) no longer animates

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Robert Longson [:longsonr]]

Apparently this example used to work in Firefox 4.

Comment 1

[Daniel Holbert [:dholbert]]

(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]]

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]]

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]]

Attachment: testcase 1 (no red should be visible)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: 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]]

(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, Feb 26~ parental leave)]

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 [:longsonr]]

So this is a duplicate of bug 658189 which also has animations with a small duration.

Comment 9

[Robert Longson [:longsonr]]

Feel free to undup if I got this wrong.

Comment 10

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(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, Feb 26~ parental leave)]

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 [:longsonr]]

We could store times as doubles perhaps?

Comment 13

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(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, Feb 26~ parental leave)]

Filed bug 755603 for fixing the frozen to-animation behaviour.

Comment 15

[Robert Longson [:longsonr]]

Attachment: Bug 733744 - don't round short durations to zero r=birtles

Comment 16

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/jobs?repo=try&revision=865fb6c04760c95a7fa21b4450500e22d68c00b6

Comment 17

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/jobs?repo=try&revision=05ad2356ab4d7bca50ccd39b4f9cf872a842058c

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a4a406091cd
don't round short durations to zero r=birtles

Comment 19

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/4a4a406091cd