I think the 'extra frame' is kind of an historical raisin - though I
cant now seem to remember why it is there :/ Though I remember some
discussion on bugzilla awhile back.

Can you post a bug on bugzilla with a proper patch (and you may need to
check that some of the behaviour math is still right).

  == Matthew

On Thu, 2008-01-31 at 20:28 +0100, benoar wrote:
> Hi,
> 
> I may have spotted a bug on how the frame number is managed in a timeline, 
> but I first would like to know how you, developers of clutter, think it 
> should work.
> 
> A timeline has a current_frame_num variable which tells the current frame 
> number. To me, it should be included in the interval [0,n_frames-1], do you 
> agree ? Because a lot of tests in clutter-timeline.c let this variable go to 
> n_frames. Most of the test are on "current_frame_num > n_frames", which is 
> wrong if you don't want current_frame_num to be equal to n_frames.
> 
> I can't do a diff for now, sorry, but here is what I spotted :
> 
> line 550:
>        (priv->current_frame_num > priv->n_frames)) ||
> Should be:
>        (priv->current_frame_num >= priv->n_frames)) ||
> line 556:
>           priv->current_frame_num = priv->n_frames;
> Should be:
>           priv->current_frame_num = priv->n_frames-1;
> line 762:
>     clutter_timeline_advance (timeline, priv->n_frames);
> Should be (well, I'm not sure exactly what should be the rule when going 
> backward):
>     clutter_timeline_advance (timeline, priv->n_frames-1);
> And line 817: how does CLAMP behave exactly ? Does it clamp to n_frames or 
> n_frames-1 ?
> 
> It is strange that this kind of error has not be spotted earlier, so maybe 
> these particular tests were intentional. But the convention in C is for the 
> "indexes" to be from 0 to max_count-1, isn't it ?
> 
> Cheers,
> Benjamin
> 
> PS: I also spotted another bug, a double increment, line 782: it should be 
> removed:
>   priv->current_frame_num += n_frames;
> 
> 

-- 
To unsubscribe send a mail to [EMAIL PROTECTED]

Reply via email to