[clutter] How are frame numbers handled in a timeline ?

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 clutter+unsubscribe_at_o-hand.com