GNOME Bugzilla – Bug 660307
patches animation.hg
Last modified: 2014-04-15 07:44:10 UTC
Created attachment 197619 [details] [review] patches animation.hg animation.hg required a templated wrap for bind() and bind_interval. Also added a refreturn to get_timeline a refreturn to get_alpha added documentation for bind(), bind_interval(), and the class.
Review of attachment 197619 [details] [review]: Sorry for taking so long to even notice that this patch was here. ::: clutter/src/animation.hg @@ +54,2 @@ _WRAP_METHOD(void set_alpha(const Glib::RefPtr<Alpha>& alpha), clutter_animation_set_alpha) + _WRAP_METHOD(Glib::RefPtr<Alpha> get_alpha() const, clutter_animation_get_alpha, refreturn) These should be in a separate patch ideally. @@ +56,3 @@ _WRAP_METHOD(void completed(), clutter_animation_completed) + _IGNORE(clutter_animation_bind, clutter_animation_bind_interval) + /** A blank line before the comment would be nice. @@ +62,3 @@ + * Given a property to bind, attaches an end value to it and + * adds it to the animation. + * @param property_name : the recognised name for the property. You don't need the : with @param. @@ +66,3 @@ + * is the programmers responsibility to ensure this is an + * appropriate type. + * @return A RefPtr to the Animation. We don't generally bother mentioning the "A RefPtr to the" part in documentation. That would get tedious quickly, @@ +85,3 @@ + Glib::RefPtr<Animation> bind_interval( + Glib::ustring property_name, + const ValueType& start_value, ustring should be passed by const reference. @@ +110,3 @@ +template <class ValueType> + Glib::RefPtr<Animation> Animation::bind( In other places in gtkmm, we let gmmroc generate, for instance, a bind_value() to directly use the C function and then add a smaller bind<>() template method that calls it. That leaves less hand-written code in the template. For instance, thought this uses Variant rather than Value: https://git.gnome.org/browse/glibmm/tree/gio/src/action.hg#n238 I think I'd prefer that. @@ +156,3 @@ + Glib::RefPtr<Clutter::Animation> pWrapper = + Glib::RefPtr<Clutter::Animation> (this); + reference(); This reference() seems odd. It needs an explanatory comment at least.
OK, thanks. I worked on this a good while back. From memory Chris Kuhl was maintaining it back then, and I got onto something else before I could get it addressed. The animation framework ends up using a fair amount of templating, because in C the functions accept any value; in C++ the type of the property creates problems making a usable interface. I don't know how best to address this, as the framework is too generalised- in particular, it's possible to animate boolean values. I'm not working on this file any more; the animation API has all changed, and the Animation and Animator classes are deprecated for either easing functions within Actor or the Animatable interface. However, I'll take the comments on board when I get to the classes that use it.
Let's forget about this because ClutterAnimation is deprecated now anyway.