box-layout: Fix allocation brain farts

The allocation code for BoxLayout contains a sequence of brain farts
that make it barely working since the synchronization of the layout
algorithm to the one in GtkBox.

The origin of the layout is inverted, and it doesn't take into
consideration a modified allocation origin (for actors the provide
padding or margin).

The pack-start property is broken, and it only works because we walk the
children list backwards; this horribly breaks when a child changes
visibility. Plus, we count invisible children, which leads to
allocations getting insane origins (either close to -MAX_FLOAT or
MAX_FLOAT).

Finally, the allocation is applied twice even for non-animated cases.

https://bugzilla.gnome.org/show_bug.cgi?id=669291

1 files changed, 21 insertions, 24 deletions

diff --git a/clutter/clutter-box-layout.c b/clutter/clutter-box-layout.c
index 8a7c8ad26..9e063864f 100644
--- a/clutter/clutter-box-layout.c
+++ b/clutter/clutter-box-layout.c
@@ -673,7 +673,7 @@ allocate_box_child (ClutterBoxLayout       *self,
           box_child->last_allocation = final_child_box;
           box_child->has_last_allocation = TRUE;
 
-          goto do_allocate;
+          return;
         }
 
       start = &box_child->last_allocation;
@@ -693,6 +693,8 @@ allocate_box_child (ClutterBoxLayout       *self,
                     final_child_box.x2, final_child_box.y2,
                     end.x1, end.y1,
                     end.x2, end.y2);
+
+      clutter_actor_allocate (child, &final_child_box, flags);
     }
   else
     {
@@ -700,9 +702,6 @@ allocate_box_child (ClutterBoxLayout       *self,
       box_child->last_allocation = final_child_box;
       box_child->has_last_allocation = TRUE;
     }
-
-do_allocate:
-  clutter_actor_allocate (child, &final_child_box, flags);
 }
 
 static void
@@ -929,6 +928,7 @@ clutter_box_layout_allocate (ClutterLayoutManager   *layout,
 
   /* Retrieve desired size for visible children. */
   children = clutter_container_get_children (container);
+
   for (i = 0, l = children; l != NULL; l = l->next)
     {
       child = l->data;
@@ -972,7 +972,6 @@ clutter_box_layout_allocate (ClutterLayoutManager   *layout,
 
       i++;
     }
-  g_list_free (children);
 
   if (priv->is_homogeneous)
     {
@@ -1017,27 +1016,24 @@ clutter_box_layout_allocate (ClutterLayoutManager   *layout,
   /* Allocate child positions. */
   if (priv->is_vertical)
     {
-      child_allocation.x1 = 0.0;
+      child_allocation.x1 = box->x1;
       child_allocation.x2 = MAX (1.0, box->x2 - box->x1);
       if (priv->is_pack_start)
-        y = 0.0;
-      else
         y = box->y2 - box->y1;
+      else
+        y = box->y1;
     }
   else
     {
-      child_allocation.y1 = 0.0;
+      child_allocation.y1 = box->y1;
       child_allocation.y2 = MAX (1.0, box->y2 - box->y1);
       if (priv->is_pack_start)
-        x = 0.0;
+        x = box->x2 - box->x1;
       else
-        x = 0.0 + box->x2 - box->x1;
+        x = box->x1;
     }
 
-  children = clutter_container_get_children (container);
-  for (i = g_list_length (children) - 1, l = g_list_last (children);
-       l != NULL;
-       l = l->prev, i--)
+  for (l = children, i = 0; l != NULL; l = l->next, i += 1)
     {
       ClutterLayoutMeta *meta;
       ClutterBoxChild *box_child;
@@ -1095,22 +1091,22 @@ clutter_box_layout_allocate (ClutterLayoutManager   *layout,
 
           if (priv->is_pack_start)
             {
-              y += child_size + priv->spacing;
-            }
-          else
-            {
               y -= child_size + priv->spacing;
 
               child_allocation.y1 -= child_size;
               child_allocation.y2 -= child_size;
             }
+          else
+            {
+              y += child_size + priv->spacing;
+            }
         }
       else /* !priv->is_vertical */
         {
           if (box_child->x_fill)
             {
               child_allocation.x1 = x;
-              child_allocation.x2 = child_allocation.x1 + MAX (1, child_size);
+              child_allocation.x2 = child_allocation.x1 + MAX (1.0, child_size);
             }
           else
             {
@@ -1120,15 +1116,15 @@ clutter_box_layout_allocate (ClutterLayoutManager   *layout,
 
           if (priv->is_pack_start)
             {
-              x += child_size + priv->spacing;
-            }
-          else
-            {
               x -= child_size + priv->spacing;
 
               child_allocation.x1 -= child_size;
               child_allocation.x2 -= child_size;
             }
+          else
+            {
+              x += child_size + priv->spacing;
+            }
 
           if (is_rtl)
             {
@@ -1146,6 +1142,7 @@ clutter_box_layout_allocate (ClutterLayoutManager   *layout,
                             &child_allocation,
                             flags);
     }
+
   g_list_free (children);
 }