Hi,
I had a short discussion with ebassi on #clutter... we decided I would send an
email detailing what I feel are some weaknesses in the Animation
API/implementation. Thanks for considering this; my thoughts follow:
----
An Animation currently keeps track of:
timeline
mode
duration
loop
However, these four properties are actually properties of:
i) The underlying Alpha object (mode, timeline)
ii) The Alpha object's timeline (duration, loop)
I am free to manipulate the Alpha object directly; I can call
clutter_animation_get_alpha(...) and then set the Alpha properties directly.
PROBLEM 1: The Animation does not watch the Alpha object for property changes,
so direct manipulation of the Alpha's properties results in the Animation
becoming out of sync with its Alpha. Concretely, this means that the
Animation's timeline no longer mirrors the Alpha's timeline and likewise for the
mode.
POTENTIAL SOLUTION: Watch Alpha for property changes and update Animation
accordingly.
BUT: Why bother mirroring the properties?
BETTER SOUTION: Just defer to the Alpha when querying/updating Animation
properties...
----
The same goes for the timeline.
PROBLEM 2: I can manipulate the timeline externally, but the Animation does not
watch for changes to timeline properties so the Timeline and Animation can get
out of sync.
POTENTIAL SOLUTION: Watch Timeline for property changes and update Animation
accordingly...
BUT: Again, why bother with the mirroring?
BETTER SOLUTION: Just fall through to the Alpha's Timeline for
querying/updating properties.
----
As it stands today, when I call clutter_animation_set_timeline(animation,
timeline), then the timeline will be stored in both the Animation and the Alpha.
PROBLEM 3: The Alpha may have been set externally (e.g. with
_animation_set_alpha(anim, alpha))... it should be made explicitly clear that
setting the Animation's timeline has the side-effect of setting the Alpha's
timeline. (This may surprise the user).
POTENTIAL SOLUTION: Document this.
BUT: Again, why bother storing the timeline in two places?
BETTER SOLUTION: Document explicitly that that animation_set_timeline is just a
helper function that sets the timeline on the underlying alpha. This way, there
are no surprises.
----
The API currently requires something like the following (note here the two calls
to _set_timeline and _set_alpha with parameter NULL).
anim = clutter_animation_new();
clutter_animation_set_object(anim, G_OBJECT(actor));
clutter_animation_set_duration(anim, 150);
clutter_animation_set_mode(anim, CLUTTER_LINEAR);
clutter_animation_set_timeline(anim, NULL);
clutter_animation_set_alpha(anim, NULL);
interval = clutter_interval_new(G_TYPE_UCHAR, 0, 255);
clutter_animation_bind_interval(anim, "opacity", interval);
timeline = clutter_animation_get_timeline(anim);
PROBLEM: If I forget to call _set_alpha, then the animation just silently fails
to do anything. (I ran into this... took me a while to figure out).
SOLUTION: An animation without an alpha is pretty useless... if the animation
has no alpha when the user tries to run it, then we should set up a default
alpha for him. This ties into the following point...
----
PROBLEM: The two calls to _set_timeline and _set_alpha with parameter NULL
should not be necessary as they are just invoking a default configuration.
SOLUTION: Make clutter_animation_get_timeline() do the right thing by creating
the necessary underlying Alpha and Timeline if they do not already exist. Then
these two calls can go away.
NOTE: (Given this solution) Generally, the underlying Alpha and Timeline will
already exist before clutter_animation_get_timeline is called because they will
need to be created in order to hold the other property values... see the notes
that follow:
NOTES:
_set_duration: requires a timeline, so this function will implicity create an
alpha and a timeline if they are not already set.
_set_loop: requires a timeline, so same as for _set_duration
_set_mode: requires an alpha, so this function will implicitly create an alpha
if one is not already set.
_set_timeline: requires an Alpha so this function will implicity create an
Alpha is one is not already set.
Finally:
_get_timeline: should make sure that there is an alpha and that it has a
timeline -- default instances can be created if they do not already exist. Then
this function ALWAYS returns a timeline and the animation is ready to run.
----
We need to document that _set_duration, _set_loop, and _set_mode are helper
functions that manipulate the underlying objects directly, just so there are no
surprises.
----
I think a helper function like clutter_animation_start() might be nice... it
would essentially just grab the timeline and run it.
----
If I understand correctly, per-property easing modes means that each property
gets its own Alpha. There would be a default Alpha with a default Timeline
(these are the ones created automatically in my solution above) that properties
would use if no Alpha/Timeline is set explicitly. This fits well with the above
proposals.
----
The key simplification to all of the above is that Animations, Alphas, and
Timelines become truly distinct objects with a clear hierarchy that can be
freely manipulated with the expected results all-around. Animations becomes
more of a "high-level" API as it is really just wrapping the "low-level"
underlying objects with a bit of glue that maps alpha updates to property value
updates.
I hope this makes some sense and I look forward to your comments. I'm happy to
provide clarification where things are less than clear.
Best regards,
Jonas Bonn
South Pole AB
--
To unsubscribe send a mail to [email protected]
