Created attachment 215526 [details] [review]
Add ClutterGridLayout

Created attachment 215527 [details] [review]
examples: Add a grid-layout example

A fairly complete example for ClutterGridLayout

Review of attachment 215526 [details] [review]:

thanks ever so much for doing this; it looks generally okay, apart from a couple of issues.

the header is missing the copyright and license notice, and the functions are missing the CLUTTER_AVAILABLE_IN_1_12 annotation.

::: clutter/clutter-grid-layout.c
@@ +52,3 @@
+ * rows and columns. It is a very similar to #ClutterTableLayout and
+ * #ClutterBoxLayout, but it consistently uses #ClutterActor's
+ * #ClutterActor:x-align and #ClutterActor:x-expand properties instead of

I'd use "it consistently uses #ClutterActor's alignment and expansion flags".

@@ +58,3 @@
+ * multiple rows or columns. It is also possible to add a child next to an
+ * existing child, using clutter_grid_layout_attach_next_to(). The behaviour of
+ * #ClutterGridLayout when several children occupy the same grid cell is undefined.

unlike gtk+, we can actually provide some guarantee on the order of painting, as well as painting multiple actors.

@@ +1540,3 @@
+  layout_class->get_child_meta_type = clutter_grid_layout_get_child_meta_type;
+
+  obj_props[PROP_ORIENTATION] =

missing annotation for the ClutterGridLayout properties.

Review of attachment 215527 [details] [review]:

looks okay

(In reply to comment #3)
> Review of attachment 215526 [details] [review]:
> 
> thanks ever so much for doing this; it looks generally okay, apart from a
> couple of issues.

you're welcome

> the header is missing the copyright and license notice, and the functions are
> missing the CLUTTER_AVAILABLE_IN_1_12 annotation.

oops, fixed

> + * #ClutterBoxLayout, but it consistently uses #ClutterActor's
> + * #ClutterActor:x-align and #ClutterActor:x-expand properties instead of
> 
> I'd use "it consistently uses #ClutterActor's alignment and expansion flags".

ok, I changed that

> + * existing child, using clutter_grid_layout_attach_next_to(). The behaviour
> of
> + * #ClutterGridLayout when several children occupy the same grid cell is
> undefined.
> 
> unlike gtk+, we can actually provide some guarantee on the order of painting,
> as well as painting multiple actors.

right, first come, first serve :)

> missing annotation for the ClutterGridLayout properties.

fixed

I'll attach a modified version of the patch for a further review

Created attachment 215554 [details] [review]
Add ClutterGridLayout

ClutterGridLayout is port of GtkGrid to a Clutter layout manager. All
the logic is taken from gtkgrid.c, so all the credits should go to
Mathias Clasen for writing this nice piece of code.

ClutterGridLayout supports adding children with it's own methods
GridLayout.attach() and GridLayout.attach_next_to() as well as
Actor.add_child() and friends. The latter adds children in a similar
fashion to ClutterBoxLayout

Review of attachment 215554 [details] [review]:

looks okay, two minor issues still remaining that should be fixed before commit.

::: clutter/clutter-grid-layout.h
@@ +76,3 @@
+{
+  /*< private >*/
+  ClutterLayoutManagerClass parent_class;

missing padding in the Class structure. I'd go for 8 slots.

@@ +79,3 @@
+};
+
+GType clutter_grid_layout_get_type (void) G_GNUC_CONST;

missing CLUTTER_AVAILABLE_IN_1_12

@@ +81,3 @@
+GType clutter_grid_layout_get_type (void) G_GNUC_CONST;
+
+ClutterLayoutManager *  clutter_grid_layout_new                 (void);

missing CLUTTER_AVAILABLE_IN_1_12

Review of attachment 215554 [details] [review]:

::: clutter/clutter-grid-layout.c
@@ +1267,3 @@
+
+  priv->container = container;
+

like BoxLayout does, the manager should also change the request mode of the container if necessary.
I'll add this too

::: clutter/clutter-grid-layout.h
@@ +79,3 @@
+};
+
+GType clutter_grid_layout_get_type (void) G_GNUC_CONST;

I either need more sleep or more coffee

Attachment 215554 [details] pushed as 1eb869e - Add ClutterGridLayout