popupMenu: Simplify allocation code

Use ClutterActor.allocate_align_fill() so we don't have to do
this math ourselves. At the same time, clean up the RTL handling
so that it's easier to follow.

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

js/ui/popupMenu.js

@@ -268,6 +268,7 @@ const PopupBaseMenuItem = new Lang.Class({
     },
 
     _allocate: function(actor, box, flags) {
+        let width = box.x2 - box.x1;
         let height = box.y2 - box.y1;
         let direction = this.actor.get_text_direction();
 
@@ -290,86 +291,46 @@ const PopupBaseMenuItem = new Lang.Class({
 
         this._ornamentLabel.allocate(ornamentBox, flags);
 
-        let x;
+        let x = box.x1;
-        if (direction == Clutter.TextDirection.LTR)
-            x = box.x1;
-        else
-            x = box.x2;
-        // if direction is ltr, x is the right edge of the last added
-        // actor, and it's constantly increasing, whereas if rtl, x is
-        // the left edge and it decreases
         for (let i = 0, col = 0; i < this._children.length; i++) {
             let child = this._children[i];
             let childBox = new Clutter.ActorBox();
 
             let [minWidth, naturalWidth] = child.actor.get_preferred_width(-1);
-            let availWidth, extraWidth;
+
-            if (this._columnWidths) {
+            let availWidth;
-                if (child.span == -1) {
+            if (child.span == -1) {
-                    if (direction == Clutter.TextDirection.LTR)
+                availWidth = box.x2 - x;
-                        availWidth = box.x2 - x;
+            } else {
-                    else
+                if (this._columnWidths) {
-                        availWidth = x - box.x1;
-                } else {
                     availWidth = 0;
                     for (let j = 0; j < child.span; j++)
                         availWidth += this._columnWidths[col++];
-                }
-                extraWidth = availWidth - naturalWidth;
-            } else {
-                if (child.span == -1) {
-                    if (direction == Clutter.TextDirection.LTR)
-                        availWidth = box.x2 - x;
-                    else
-                        availWidth = x - box.x1;
                 } else {
                     availWidth = naturalWidth;
                 }
-                extraWidth = 0;
             }
 
-            if (direction == Clutter.TextDirection.LTR) {
+            if (direction == Clutter.TextDirection.RTL)
-                if (child.expand) {
+                childBox.x1 = width - x - availWidth;
-                    childBox.x1 = x;
+            else
-                    childBox.x2 = x + availWidth;
+                childBox.x1 = x;
-                } else if (child.align === St.Align.MIDDLE) {
+            childBox.x2 = childBox.x1 + availWidth;
-                    childBox.x1 = x + Math.round(extraWidth / 2);
-                    childBox.x2 = childBox.x1 + naturalWidth;
-                } else if (child.align === St.Align.END) {
-                    childBox.x2 = x + availWidth;
-                    childBox.x1 = childBox.x2 - naturalWidth;
-                } else {
-                    childBox.x1 = x;
-                    childBox.x2 = x + naturalWidth;
-                }
-            } else {
-                if (child.expand) {
-                    childBox.x1 = x - availWidth;
-                    childBox.x2 = x;
-                } else if (child.align === St.Align.MIDDLE) {
-                    childBox.x1 = x - Math.round(extraWidth / 2);
-                    childBox.x2 = childBox.x1 + naturalWidth;
-                } else if (child.align === St.Align.END) {
-                    // align to the left
-                    childBox.x1 = x - availWidth;
-                    childBox.x2 = childBox.x1 + naturalWidth;
-                } else {
-                    // align to the right
-                    childBox.x2 = x;
-                    childBox.x1 = x - naturalWidth;
-                }
-            }
 
             let [minHeight, naturalHeight] = child.actor.get_preferred_height(childBox.x2 - childBox.x1);
             childBox.y1 = Math.round(box.y1 + (height - naturalHeight) / 2);
             childBox.y2 = childBox.y1 + naturalHeight;
 
-            child.actor.allocate(childBox, flags);
+            let [xAlign, yAlign] = St.get_align_factors(child.align,
+                                                        St.Align.MIDDLE);
 
-            if (direction == Clutter.TextDirection.LTR)
+            // It's called "expand", but it's really more like "fill"
-                x += availWidth + this._spacing;
+            let xFill = child.expand;
-            else
+            child.actor.allocate_align_fill(childBox,
-                x -= availWidth + this._spacing;
+                                            xAlign, yAlign,
+                                            xFill, true,
+                                            flags);
+            x += availWidth + this._spacing;
         }
     }
 });

src/shell-slicer.c

@@ -104,8 +104,8 @@ shell_slicer_paint_child (ShellSlicer *self)
     return;
 
   st_bin_get_alignment (ST_BIN (self), &x_align, &y_align);
-  _st_get_align_factors (x_align, y_align,
+  st_get_align_factors (x_align, y_align,
-                         &x_align_factor, &y_align_factor);
+                        &x_align_factor, &y_align_factor);
 
   clutter_actor_get_allocation_box (CLUTTER_ACTOR (self), &self_box);
   clutter_actor_get_allocation_box (child, &child_box);

src/st/st-bin.c

@@ -108,8 +108,8 @@ st_bin_allocate (ClutterActor          *self,
       gdouble x_align_f, y_align_f;
 
       st_theme_node_get_content_box (theme_node, box, &childbox);
-      _st_get_align_factors (priv->x_align, priv->y_align,
+      st_get_align_factors (priv->x_align, priv->y_align,
-                             &x_align_f, &y_align_f);
+                            &x_align_f, &y_align_f);
       clutter_actor_allocate_align_fill (priv->child, &childbox,
                                          x_align_f, y_align_f,
                                          priv->x_fill, priv->y_fill,

src/st/st-box-layout.c

@@ -736,7 +736,7 @@ st_box_layout_allocate (ClutterActor          *actor,
                                    "expand", &expand,
                                    NULL);
 
-      _st_get_align_factors (xalign, yalign, &xalign_f, &yalign_f);
+      st_get_align_factors (xalign, yalign, &xalign_f, &yalign_f);
 
       if (priv->is_vertical)
         {

src/st/st-private.c

@@ -98,66 +98,6 @@ _st_actor_get_preferred_height (ClutterActor *actor,
   clutter_actor_get_preferred_height (actor, for_width, min_height_p, natural_height_p);
 }
 
-/**
- * _st_get_align_factors:
- * @x_align: an #StAlign
- * @y_align: an #StAlign
- * @x_align_out: (out) (allow-none): @x_align as a #gdouble
- * @y_align_out: (out) (allow-none): @y_align as a #gdouble
- *
- * Converts @x_align and @y_align to #gdouble values.
- */
-void
-_st_get_align_factors (StAlign   x_align,
-                       StAlign   y_align,
-                       gdouble  *x_align_out,
-                       gdouble  *y_align_out)
-{
-  if (x_align_out)
-    {
-      switch (x_align)
-        {
-        case ST_ALIGN_START:
-          *x_align_out = 0.0;
-          break;
-
-        case ST_ALIGN_MIDDLE:
-          *x_align_out = 0.5;
-          break;
-
-        case ST_ALIGN_END:
-          *x_align_out = 1.0;
-          break;
-
-        default:
-          g_warn_if_reached ();
-          break;
-        }
-    }
-
-  if (y_align_out)
-    {
-      switch (y_align)
-        {
-        case ST_ALIGN_START:
-          *y_align_out = 0.0;
-          break;
-
-        case ST_ALIGN_MIDDLE:
-          *y_align_out = 0.5;
-          break;
-
-        case ST_ALIGN_END:
-          *y_align_out = 1.0;
-          break;
-
-        default:
-          g_warn_if_reached ();
-          break;
-        }
-    }
-}
-
 /**
  * _st_set_text_from_style:
  * @text: Target #ClutterText

src/st/st-private.h

@@ -45,11 +45,6 @@ G_END_DECLS
 
 ClutterActor *_st_widget_get_dnd_clone (StWidget *widget);
 
-void _st_get_align_factors (StAlign   x_align,
-                            StAlign   y_align,
-                            gdouble  *x_align_out,
-                            gdouble  *y_align_out);
-
 void _st_actor_get_preferred_width  (ClutterActor *actor,
                                      gfloat        for_height,
                                      gboolean      y_fill,

src/st/st-table.c

@@ -257,8 +257,8 @@ st_table_homogeneous_allocate (ClutterActor          *self,
       row_span = meta->row_span;
       col_span = meta->col_span;
 
-      _st_get_align_factors (meta->x_align, meta->y_align,
+      st_get_align_factors (meta->x_align, meta->y_align,
-                             &x_align_f, &y_align_f);
+                            &x_align_f, &y_align_f);
 
       if (ltr)
         {
@@ -609,8 +609,8 @@ st_table_preferred_allocate (ClutterActor          *self,
       row_span = meta->row_span;
       col_span = meta->col_span;
 
-      _st_get_align_factors (meta->x_align, meta->y_align,
+      st_get_align_factors (meta->x_align, meta->y_align,
-                             &x_align_f, &y_align_f);
+                            &x_align_f, &y_align_f);
 
       /* initialise the width and height */
       col_width = col_widths[col];

src/st/st-widget.c

@@ -2885,3 +2885,63 @@ st_widget_get_focus_chain (StWidget *widget)
 {
   return ST_WIDGET_GET_CLASS (widget)->get_focus_chain (widget);
 }
+
+/**
+ * st_get_align_factors:
+ * @x_align: an #StAlign
+ * @y_align: an #StAlign
+ * @x_align_out: (out) (allow-none): @x_align as a #gdouble
+ * @y_align_out: (out) (allow-none): @y_align as a #gdouble
+ *
+ * Converts @x_align and @y_align to #gdouble values.
+ */
+void
+st_get_align_factors (StAlign   x_align,
+                      StAlign   y_align,
+                      gdouble  *x_align_out,
+                      gdouble  *y_align_out)
+{
+  if (x_align_out)
+    {
+      switch (x_align)
+        {
+        case ST_ALIGN_START:
+          *x_align_out = 0.0;
+          break;
+
+        case ST_ALIGN_MIDDLE:
+          *x_align_out = 0.5;
+          break;
+
+        case ST_ALIGN_END:
+          *x_align_out = 1.0;
+          break;
+
+        default:
+          g_warn_if_reached ();
+          break;
+        }
+    }
+
+  if (y_align_out)
+    {
+      switch (y_align)
+        {
+        case ST_ALIGN_START:
+          *y_align_out = 0.0;
+          break;
+
+        case ST_ALIGN_MIDDLE:
+          *y_align_out = 0.5;
+          break;
+
+        case ST_ALIGN_END:
+          *y_align_out = 1.0;
+          break;
+
+        default:
+          g_warn_if_reached ();
+          break;
+        }
+    }
+}

src/st/st-widget.h

@@ -151,7 +151,6 @@ StThemeNode *         st_widget_peek_theme_node           (StWidget        *widg
 GList *               st_widget_get_focus_chain           (StWidget        *widget);
 void                  st_widget_paint_background          (StWidget        *widget);
 
-
 /* debug methods */
 char  *st_describe_actor       (ClutterActor *actor);
 void   st_set_slow_down_factor (gfloat factor);
@@ -169,6 +168,12 @@ void                  st_widget_set_accessible_name      (StWidget    *widget,
                                                           const gchar *name);
 const gchar *         st_widget_get_accessible_name      (StWidget    *widget);
 
+/* utility methods */
+void st_get_align_factors (StAlign   x_align,
+                           StAlign   y_align,
+                           gdouble  *x_align_out,
+                           gdouble  *y_align_out);
+
 G_END_DECLS
 
 #endif /* __ST_WIDGET_H__ */