On Wed, 2009-05-27 at 17:18 +0200, Jonas Bonn wrote:

> > which is basically what are you proposing, except that I wouldn't remove
> > the loop, duration and mode variables from AnimationPrivate.
> > 
> 
> There is some confusion here involving the caching.  The right way would be 
> either:
> 
> i) the variables are cached and used at object creation time,
> or
> ii) the objects are created when the variables are read/written and the 
> values 
> are proxied.
> 
> What you have below is a mix of the two (explained below).
> 
> My preference is to proxy the values.  Is there any advantage to caching them?

the more I think about this, the more:

  set_alpha(NULL)
  set_timeline(NULL)

for creating defaults are something that I would not want to expose in
the API.

with those gone from the API/arguments matrix all that remains is:

  set_mode(), get_mode(): create alpha
  set_duration(), get_duration(): create alpha, timeline
  set_loop(), get_loop(): create alpha, timeline

  set_alpha(): sets alpha
  get_alpha(): creates alpha, if needed

  set_timeline(): creates alpha if needed, sets timeline
  get_timeline(): creates alpha, timeline if needed

> The reason for this is that the user may want to "disconnect" his alpha from 
> his 
> animation in order to use the alpha to drive something else without having 
> the 
> animation run.

I'm not overly sold on this. an Alpha is a thin wrapper around a
timeline and an easing mode -- if you need to do this then you're better
off destroying the animation.

let's assume allowing to unset the alpha -- to what would map:

  set_mode(CLUTTER_LINEAR);
  set_timeline(NULL);

? I guess it would just call alpha_set_timeline(NULL), which is a valid
behaviour. oh well, it wouldn't hurt, I guess, even though it would be
pretty much insane.

> Of course, the advantage that I see with you approach is compatibility with 
> code 
> already written for 0.9... but the API is unstable, so this minor breakage 
> shouldn't be a big deal (the fix involves code removal... pretty easy).

the amount if code using Animation directly should not be large, so that
wouldn't be a problem. I'd be more worried about changes in the
behaviour of clutter_actor_animate*(), but those should not break as far
as my checks go.

> What all of the above boils down to is the API effectively has two patterns:
> 
> _animation_new()
> _set_mode()
> _set_duration()
> _set_loop()
> get_timeline
> 
> and
> 
> _animation_new()
> _set_alpha( EXTERNAL ALPHA )
> _get_timeline

yeah, I agree on both use cases.

so, apart from the extra-rambly email, I went through three iterations
of the patch and at the end I have something similar to your initial
approach - plus the removal of the automagic creation of alpha and
timeline if NULL is passed to set_alpha() and set_timeline()
respectively and some more documentation. you can find it inside the
1.0-integration branch, which will be merged into master before the API
frozen release.

ciao,
 Emmanuele.

-- 
Emmanuele Bassi, Senior Engineer        | [email protected]
Intel Open Source Technology Center     | http://oss.intel.com

-- 
To unsubscribe send a mail to [email protected]

Reply via email to