You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by Justin Mclean <ju...@classsoftware.com> on 2013/12/16 12:55:04 UTC
Interesting animation bug
Hi,
Here's a curious one:
https://issues.apache.org/jira/browse/FLEX-33974
From what I can see there's a couple of bugs in there.
The most obvious one being:
public function get playheadTime():Number
{
- return _playheadTime + startDelay;
+ return _playheadTime;
}
And the other fix being updating _playheadTiem not being updated before the start time of the animation:
// Keep starting animations unless our sorted lists return
// animations that start past the current time
if (animStartTime < Timeline.currentTime)
if (anim.playReversed)
anim.end();
else
anim.start();
else
+ anim._playheadTime = intervalTime;
break;
However with this change we get about 20 effect tests failing, mostly in repeat count/repeat delay style tests. It looks to me like the repeat code was previously incorrect as duration already included startDelay.
numRepeats += (_playheadTime - duration) / (duration + repeatDelay);
See the original playheadTime above. Anyone see something I've missed or are the current tests possibly testing current incorrect behaviour?
Thanks,
Justin
Re: Interesting animation bug
Posted by Alex Harui <ah...@adobe.com>.
On 12/16/13 10:08 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> In one case start method is called right away, in the other only after
>>the startDelay has passed.
>But then I guess the question is when should start be called at time zero
>or after start delay?
IMO, start delay should delay the start of the effect, cycleTime should
then be zero and playheadTime would be startDelay
-Alex
Re: Interesting animation bug
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> In one case start method is called right away, in the other only after the startDelay has passed.
But then I guess the question is when should start be called at time zero or after start delay?
Justin
Re: Interesting animation bug
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Maybe worth further investigation?
In one case start method is called right away, in the other only after the startDelay has passed. Looks like startDelay isn't passed down in the first case, before start being called it will return NaN.
Not 100% sure but looks like there an error in Effect.as createInstances method, where:
// Multiple target support
var n:int = targets.length;
var offsetDelay:Number = 0;
Should be:
// Multiple target support
var n:int = targets.length;
var offsetDelay:Number = startDelay;
> Interesting, the doc says it includes repetitions. I figured this number
> would keep increasing and not set back.
And that's what it does but not what I expected. I had assumed it acted more like a flash timeline.
Justin
Re: Interesting animation bug
Posted by Alex Harui <ah...@adobe.com>.
On 12/16/13 3:54 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> Do you know why Scale doesn't display NaN?
>No.
>
>> I didn't see any code setting _playheadTime to zero, but I certainly
>>could have missed it.
>Not sure either.
Maybe worth further investigation?
>
>> I think you're saying that since _playheadTime is continuously
>>increasing
>> even when waiting for startDelay, that there is no need to factor
>> startDelay into the getter. Is that correct?
>Correct but it should loop about back to that start time when repeating.
>
>eg say with a start delay of 200 and a duration of 100 - something like
>this:
>0
>50
>100
>150
>200
>250
>300
>200
>250
>300
Interesting, the doc says it includes repetitions. I figured this number
would keep increasing and not set back. Are you sure that's the expected
behavior?
-Alex
Re: Interesting animation bug
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Do you know why Scale doesn't display NaN?
No.
> I didn't see any code setting _playheadTime to zero, but I certainly could have missed it.
Not sure either.
> I think you're saying that since _playheadTime is continuously increasing
> even when waiting for startDelay, that there is no need to factor
> startDelay into the getter. Is that correct?
Correct but it should loop about back to that start time when repeating.
eg say with a start delay of 200 and a duration of 100 - something like this:
0
50
100
150
200
250
300
200
250
300
etc
etc
Thanks,
Justin
Re: Interesting animation bug
Posted by Alex Harui <ah...@adobe.com>.
Do you know why Scale doesn't display NaN? I didn't see any code setting
_playheadTime to zero, but I certainly could have missed it.
I think you're saying that since _playheadTime is continuously increasing
even when waiting for startDelay, that there is no need to factor
startDelay into the getter. Is that correct?
-Alex
On 12/16/13 2:39 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> In the bug, the user seems to want to see time increasing while time <
>> startDelay.
>More about having it consistent between the two types of animation,
>notice that both has a startDelay of 200 but return different values in
>that time interval.
>
>> Is that what you think should happen too, or should it be 0 or
>> NaN?
>IMO 0 initially as it 0 after it finished not NaN.
>
>> If it should just increase, I don't understand the point of
>> including startDelay.
>It should include the start delay period and increase IMO just as Scale
>does.
>
>Thanks,
>Justin
Re: Interesting animation bug
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> In the bug, the user seems to want to see time increasing while time <
> startDelay.
More about having it consistent between the two types of animation, notice that both has a startDelay of 200 but return different values in that time interval.
> Is that what you think should happen too, or should it be 0 or
> NaN?
IMO 0 initially as it 0 after it finished not NaN.
> If it should just increase, I don't understand the point of
> including startDelay.
It should include the start delay period and increase IMO just as Scale does.
Thanks,
Justin
Re: Interesting animation bug
Posted by Alex Harui <ah...@adobe.com>.
In the bug, the user seems to want to see time increasing while time <
startDelay. Is that what you think should happen too, or should it be 0 or
NaN? If it should just increase, I don't understand the point of
including startDelay.
On 12/16/13 2:22 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> It is a bit scary since the documentation took the time to mention that
>> playheadTime includes startDelay.
>Yes it should include the time from 0 up to and including startDelay, but
>it shouldn't be adding the full value of startDelay to it before it has
>happened.
>
>On a time line you have in order:
>startDelay period
>duration period
>repeatDelay period
>duration period
>repeatDelay period
>
>etc etc
>
>Also a bit scary use of _playheadTime vs playheadTime in the code as
>currently one include + startDelay and one doesn't.
>
>Justin
>
Re: Interesting animation bug
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> It is a bit scary since the documentation took the time to mention that
> playheadTime includes startDelay.
Yes it should include the time from 0 up to and including startDelay, but it shouldn't be adding the full value of startDelay to it before it has happened.
On a time line you have in order:
startDelay period
duration period
repeatDelay period
duration period
repeatDelay period
etc etc
Also a bit scary use of _playheadTime vs playheadTime in the code as currently one include + startDelay and one doesn't.
Justin
Re: Interesting animation bug
Posted by Alex Harui <ah...@adobe.com>.
It is a bit scary since the documentation took the time to mention that
playheadTime includes startDelay. I don't know this code at all, but it
makes me think that there is a reason for it.
On 12/16/13 1:12 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> Why shouldn't playheadtime include the startdelay?
>
>Because then the values returned are something like this:
>
>Elapsed Time/playheadTime
>0/200
>27/200
>65/200
>109/200
>148/200
>178/200
>232/232
>264/264
>
>It also gives different results to anything that extends AnimateTransform
>(eg Scale), which would give this:
>
>Elapsed Time/playheadTime
>0/0
>27/27
>65/65
>109/109
>148/148
>178/178
>232/232
>264/264
>
>Thanks,
>Justin
Re: Interesting animation bug
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Why shouldn't playheadtime include the startdelay?
Because then the values returned are something like this:
Elapsed Time/playheadTime
0/200
27/200
65/200
109/200
148/200
178/200
232/232
264/264
It also gives different results to anything that extends AnimateTransform (eg Scale), which would give this:
Elapsed Time/playheadTime
0/0
27/27
65/65
109/109
148/148
178/178
232/232
264/264
Thanks,
Justin
Re: Interesting animation bug
Posted by Alex Harui <ah...@adobe.com>.
Why shouldn't playheadtime include the startdelay?
-Alex
On 12/16/13 3:55 AM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>Here's a curious one:
>https://issues.apache.org/jira/browse/FLEX-33974
>
>From what I can see there's a couple of bugs in there.
>
>The most obvious one being:
> public function get playheadTime():Number
> {
>- return _playheadTime + startDelay;
>+ return _playheadTime;
> }
>
>And the other fix being updating _playheadTiem not being updated before
>the start time of the animation:
>
> // Keep starting animations unless our sorted lists return
> // animations that start past the current time
> if (animStartTime < Timeline.currentTime)
> if (anim.playReversed)
> anim.end();
> else
> anim.start();
> else
>+ anim._playheadTime = intervalTime;
> break;
>
>However with this change we get about 20 effect tests failing, mostly in
>repeat count/repeat delay style tests. It looks to me like the repeat
>code was previously incorrect as duration already included startDelay.
>
>numRepeats += (_playheadTime - duration) / (duration + repeatDelay);
>
>See the original playheadTime above. Anyone see something I've missed or
>are the current tests possibly testing current incorrect behaviour?
>
>Thanks,
>Justin
>
>
RE: Interesting animation bug
Posted by Maurice Amsellem <ma...@systar.com>.
Ok thanks
-----Message d'origine-----
De : Justin Mclean [mailto:justin@classsoftware.com]
Envoyé : lundi 16 décembre 2013 13:12
À : dev@flex.apache.org
Objet : Re: Interesting animation bug
Hi,
> Justin, I couldn't find the code you mention above.
spark/effects/Animation.as in the spark project.
Thanks,
Justin
Re: Interesting animation bug
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Justin, I couldn't find the code you mention above.
spark/effects/Animation.as in the spark project.
Thanks,
Justin
RE: Interesting animation bug
Posted by Maurice Amsellem <ma...@systar.com>.
Justin, I couldn't find the code you mention above.
In which class is it ?
Maurice
-----Message d'origine-----
De : Justin Mclean [mailto:justin@classsoftware.com]
Envoyé : lundi 16 décembre 2013 12:55
À : dev@flex.apache.org
Objet : Interesting animation bug
Hi,
Here's a curious one:
https://issues.apache.org/jira/browse/FLEX-33974
>From what I can see there's a couple of bugs in there.
The most obvious one being:
public function get playheadTime():Number
{
- return _playheadTime + startDelay;
+ return _playheadTime;
}
And the other fix being updating _playheadTiem not being updated before the start time of the animation:
// Keep starting animations unless our sorted lists return
// animations that start past the current time
if (animStartTime < Timeline.currentTime)
if (anim.playReversed)
anim.end();
else
anim.start();
else
+ anim._playheadTime = intervalTime;
break;
However with this change we get about 20 effect tests failing, mostly in repeat count/repeat delay style tests. It looks to me like the repeat code was previously incorrect as duration already included startDelay.
numRepeats += (_playheadTime - duration) / (duration + repeatDelay);
See the original playheadTime above. Anyone see something I've missed or are the current tests possibly testing current incorrect behaviour?
Thanks,
Justin
Re: Interesting animation bug
Posted by Tom Chiverton <tc...@extravision.com>.
On 16/12/2013 11:55, Justin Mclean wrote:
> numRepeats += (_playheadTime - duration) / (duration + repeatDelay);
>
> See the original playheadTime above. Anyone see something I've missed or are the current tests possibly testing current incorrect behaviour?
Well, this is an interesting one, because people will have other things
sync'ed up with the repeat count, for instance, so do we really want to
change that ?
Tom
RE: Interesting animation bug
Posted by Maurice Amsellem <ma...@systar.com>.
>as duration already included startDelay.
I did a simple test:
<s:Scale id="_scale" target="{box}" scaleXFrom="1.0" scaleXTo="2.0" duration="5000" startDelay="5000"/>
And the total duration of the animation is 5000+5000;
So it seems that duration does not include startDelay.
Is that correct?
Maurice
-----Message d'origine-----
De : Justin Mclean [mailto:justin@classsoftware.com]
Envoyé : lundi 16 décembre 2013 12:55
À : dev@flex.apache.org
Objet : Interesting animation bug
Hi,
Here's a curious one:
https://issues.apache.org/jira/browse/FLEX-33974
>From what I can see there's a couple of bugs in there.
The most obvious one being:
public function get playheadTime():Number
{
- return _playheadTime + startDelay;
+ return _playheadTime;
}
And the other fix being updating _playheadTiem not being updated before the start time of the animation:
// Keep starting animations unless our sorted lists return
// animations that start past the current time
if (animStartTime < Timeline.currentTime)
if (anim.playReversed)
anim.end();
else
anim.start();
else
+ anim._playheadTime = intervalTime;
break;
However with this change we get about 20 effect tests failing, mostly in repeat count/repeat delay style tests. It looks to me like the repeat code was previously incorrect as duration already included startDelay.
numRepeats += (_playheadTime - duration) / (duration + repeatDelay);
See the original playheadTime above. Anyone see something I've missed or are the current tests possibly testing current incorrect behaviour?
Thanks,
Justin