Hi, I think I could spend some time reviewing these patches too, if that would help; although I'm of course no expert on this.
However, I may be wrong because I just took a quick look at the bug report, but I do not see any modifications to the patches tagged as 'needs-work'. Adding classes' .hg and .ccg files without adding them to filelist.am is safe, as the build will not break, but it is kind of pointless. So I think yes, maybe creating self-contained patches would ease the process of getting them into cluttermm. Best regards, Juan. On Mon, Mar 3, 2014 at 8:29 AM, Ian Martin <martin...@vodafone.co.nz> wrote: > Hi, > I have a few questions regarding how to get the patchset accepted to update > cluttermm. > > The first is will it happen? I've had a large amount of silence after the > initial review. > > Second, I'm uncertain how much work I'd have to do to make it possible. > Murray's comments on the bug (at this bugzilla address- bug 725125 suggest > I'd have to isolate all the elements belonging to each class/ set of classes > to get the patchset approved. In particular, to identify the > clutter_methods.defs/ clutter_signals.defs changes required for each file > would mean manually going through the generated defs files and isolating the > changes for each set of components. For each new .hg file I'd also have to > add the diff for the codegen/m4/convert_clutter.m4 file, the > codegen/generate_extra_defs.cc file, the src/clutter/filelist.am file, as > well as the changes in downstream classes (Clutter::Actor in particular) > that rely on the new class. > > Third, I've now got the complex animation framework functioning, and writing > the example/ test cases identified some problems with the wrapper that I've > fixed. Should I add this as another patch to apply after all the previous > ones, or redo the patchset so that this is incorporated in the original > diff? > > Finally, a policy question. > > The complex animation framework uses the Interval class, which in C has a > constructor with the following signature: > ClutterInterval * clutter_interval_new (GType gtype, ...); > > and an alternate for wrapping libraries of > > ClutterInterval * clutter_interval_new_with_values (GType gtype, > const GValue > *initial, > const GValue > *final); > > What I propose is to wrap numeric values in the Clutter::Interval class to > allow the following: > > static Glib::RefPtr< Interval > create (const int initialv, const int > finalv) > > static Glib::RefPtr< Interval > create (const double initialv, const > double finalv) > > static Glib::RefPtr< Interval > create (const float initialv, const > float finalv) > > while keeping the Glib::Value constructor to allow use of other types if > required: > > static Glib::RefPtr< Interval > create (GType value_type, const > Glib::ValueBase& initialv, const Glib::ValueBase& finalv) > > > From a usability point of view, I find Glib::Values poxy- they require that > you know the GType before you construct one, so you have to expose the type > to the end-user. However, in this context the type of values required are > all Clutter::Actor properties, and these fall into 3 groups. Most are > numeric (either int, float or double); some are Clutter classes (e.g. > color), and then a number are bool or enums. > > Given that this is for an animation framework, animating a bool or enum > value is an oxymoron. Is it acceptable to overload the create() method to > allow ease of use of the common numeric types? > > > Ian. > > _______________________________________________ > gtkmm-list mailing list > gtkmm-list@gnome.org > https://mail.gnome.org/mailman/listinfo/gtkmm-list > _______________________________________________ gtkmm-list mailing list gtkmm-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtkmm-list
