I know this is unethical, sorry, but I'm going to paste code for reference.

The patch does not add the function cleanly as I suspected it might,
the function which handles this particular case is pasted here in line
for reference:


/* Handle the request in the opposite orientation of the box
 * (i.e. height request of horizontal box)
 *
 * This operation requires a virtual allocation in the natural
 * orientation of the box, after that each element must be asked
 * for the size-for-virtually-allocated-size and the maximums of
 * each child sample will be reported as the overall
 * "size-for-size-in-opposite-orientation"
 */
static void
get_preferred_size_for_opposite_orientation (ClutterBoxLayout   *self,
                                             ClutterActor       *container,
                                             gfloat              for_size,
                                             gfloat             *min_size_p,
                                             gfloat             *natural_size_p)
{
  ClutterLayoutManager    *layout = CLUTTER_LAYOUT_MANAGER (self);
  ClutterBoxLayoutPrivate *priv = self->priv;
  ClutterContainer        *real_container = CLUTTER_CONTAINER (container);
  ClutterActor            *child;
  gint                     nvis_children = 0, n_extra_widgets = 0;
  gint                     nexpand_children = 0, i;
  RequestedSize           *sizes;
  gfloat                   minimum, natural, size, extra = 0;
  ClutterOrientation       opposite_orientation =
    priv->orientation == CLUTTER_ORIENTATION_HORIZONTAL ? CLUTTER_ORIENTATION_VERTICAL : CLUTTER_ORIENTATION_HORIZONTAL;

  minimum = natural = 0;

  count_expand_children (layout, real_container,
                         &nvis_children, &nexpand_children);

  if (nvis_children < 1)
    {
      if (min_size_p)
        *min_size_p = 0;

      if (natural_size_p)
        *natural_size_p = 0;

      return;
    }

  /* First collect the requested sizes in the natural orientation of the box */
  sizes  = g_newa (RequestedSize, nvis_children);
  size   = for_size;

  for (i = 0, child = clutter_actor_get_first_child (container);
       child != NULL;
       child = clutter_actor_get_next_sibling (child))
    {
      if (!CLUTTER_ACTOR_IS_VISIBLE (child))
        continue;

      get_child_size (child, priv->orientation, -1, 
                      &sizes[i].minimum_size,
                      &sizes[i].natural_size);

      size -= sizes[i].minimum_size;
      i++;
    }

  if (priv->is_homogeneous)
    {
      size            = for_size - (nvis_children - 1) * priv->spacing;
      extra           = size / nvis_children;
      n_extra_widgets = ((gint)size) % nvis_children;
    }
  else
    {
      /* Bring children up to size first */
      size = distribute_natural_allocation (MAX (0, size), nvis_children, sizes);

      /* Calculate space which hasn't distributed yet,
       * and is available for expanding children.
       */
      if (nexpand_children > 0)
        {
          extra = size / nexpand_children;
          n_extra_widgets = ((gint)size) % nexpand_children;
        }
    }

  /* Distribute expand space to children */
  for (i = 0, child = clutter_actor_get_first_child (container);
       child != NULL;
       child = clutter_actor_get_next_sibling (child))
    {
      ClutterLayoutMeta *meta;
      ClutterBoxChild   *box_child;

      /* If widget is not visible, skip it. */
      if (!CLUTTER_ACTOR_IS_VISIBLE (child))
        continue;

      meta      = clutter_layout_manager_get_child_meta (layout, real_container, child);
      box_child = CLUTTER_BOX_CHILD (meta);

      if (priv->is_homogeneous)
        {
          sizes[i].minimum_size = extra;
          
          if (n_extra_widgets > 0)
            {
              sizes[i].minimum_size++;
              n_extra_widgets--;
            }
        }
      else
        {
          if (clutter_actor_needs_expand (child, priv->orientation) || box_child->expand)
            {
              sizes[i].minimum_size += extra;

              if (n_extra_widgets > 0)
                {
                  sizes[i].minimum_size++;
                  n_extra_widgets--;
                }
            }
        }
      i++;
    }

  /* Virtual allocation finished, now we can finally ask for the right size-for-size */
  for (i = 0, child = clutter_actor_get_first_child (container);
       child != NULL;
       child = clutter_actor_get_next_sibling (child))
    {
      gfloat child_min = 0, child_nat = 0;

      if (!CLUTTER_ACTOR_IS_VISIBLE (child))
        continue;

      get_child_size (child, opposite_orientation,
                      sizes[i].minimum_size,
                      &child_min, &child_nat);

      minimum = MAX (minimum, child_min);
      natural = MAX (natural, child_nat);

      i++;
    }

  if (min_size_p)
    *min_size_p = minimum;

  if (natural_size_p)
    *natural_size_p = natural;
}

Created attachment 218148 [details]
clutter script to display the problem

Here is a clutter script that shows the problem

We have:

  stage (bin layout set to center align) {
     actor (box layout horiontal, background orange) {
       long wrapping label,
       another long wrapping label
     }

  }


When displaying this attached clutter script, you can see that the box does not
allocate enough height to display the contained text.

With the patch applied, the collective height for width request passes
correctly and the text wraps up and forces the orange box to surround it as
expected.

Note: There's one more thing I would recommend with this patch; The ClutterBoxLayout should not drive the actor's 'request-mode' property,

it seems it was driving that with the thought in mind that if it were
to be allocated using it's preferred request mode by it's parent, then
it would not hit the broken request routine (which would be why so far
boxes "seem" to work fine, so long as you place them inside a fixed layout
or another free form layout like a bin layout, it breaks only when you put
a box in a box, or a box in a flow layout I suppose...).

In any case I also recommend we just remove that code that drives
the request-mode, ClutterBoxLayout can handle both requests now
and users can decide the request-mode set on the actor (which will
make a sublte difference on how box hierarchies are displayed when
placed in a fixed layout where the request-mode is not ignored for
the toplevel box...)

Review of attachment 218128 [details] [review]:

apart from minor coding style issues, it looks good to me.

::: clutter/clutter-box-layout.c
@@ +487,3 @@
+{
+  ClutterBoxLayoutPrivate *priv = self->priv;
+  ClutterActor            *child;

you can remove the whitespace here.

@@ +495,1 @@
+  for (child = clutter_actor_get_first_child (container);

I would use a ClutterActorIter.

@@ +531,3 @@
+{
+  ClutterBoxLayoutPrivate *priv = self->priv;
+  ClutterActor            *child;

same as above - you can remove the whitespace between type and variable name.

@@ +535,3 @@
+  gfloat                   minimum, natural;
+  ClutterOrientation       opposite_orientation =
+    priv->orientation == CLUTTER_ORIENTATION_HORIZONTAL ? CLUTTER_ORIENTATION_VERTICAL : CLUTTER_ORIENTATION_HORIZONTAL;

the line should be broken at '?', i.e.:

  ClutterOrientation opposite_orientation =
    priv->orientation == CLUTTER_ORIENTATION_HORIZONTAL
                       ? CLUTTER_ORIENTATION_VERTICAL
                       : CLUTTER_ORIENTATION_HORIZONTAL;

@@ +539,3 @@
+  minimum = natural = 0;
+
+  for (child = clutter_actor_get_first_child (container);

I'd use a ClutterActorIter.

@@ +589,3 @@
+  gfloat                   minimum, natural, size, extra = 0;
+  ClutterOrientation       opposite_orientation =
+    priv->orientation == CLUTTER_ORIENTATION_HORIZONTAL ? CLUTTER_ORIENTATION_VERTICAL : CLUTTER_ORIENTATION_HORIZONTAL;

same line break as above.

@@ +613,1 @@
+  for (i = 0, child = clutter_actor_get_first_child (container);

I'd use a ClutterActorIter

@@ +648,3 @@
+
+  /* Distribute expand space to children */
+  for (i = 0, child = clutter_actor_get_first_child (container);

I'd use a ClutterActorIter instead of the for() loop - this way we avoid a type check for the first iteration, and a type check for each following iteration; clutter_actor_iter_next() does not perform any type check at all.

@@ +689,3 @@
 
+  /* Virtual allocation finished, now we can finally ask for the right size-for-size */
+  for (i = 0, child = clutter_actor_get_first_child (container);

same as above.

(In reply to comment #3)
> Note: There's one more thing I would recommend with this patch; The
> ClutterBoxLayout should not drive the actor's 'request-mode' property,

if the behaviour has been fixed, then we can probably remove it, though it would probably be nice to have some visual test of what flipping the request-mode does to an actor using a BoxLayout.

Right, I can quicky illustrate what happens for the sake of noting it at least:

  a.) First, the request mode only has any effect when placed in some
      free form parent layout which allow free allocation of natural
      size.

      (so the key is either setting the desired mode at the top level
      or letting it be derived by way of competing preferences of children
      in some way, so probably always the former is best).


  b.) Sticking to 2 wrapping labels in a horizontal box; currently in the
      inverted width-for-height mode, the desired height will first be
      calculated for each wrapping clutter text

      Assuming the ClutterText is doing it's job, it's requesting enough
      height for a width of -1 at the *minimum* width that the text could
      possibly wrap up to (the text case attached seems to achieve this
      at least by way of an explicit width set).

  b1.) Then width for height is requested on the box, in this
       case the required height of the text actors *must* be a large
       height so you get the minimum possible width here as the natural
       width.


  c.) Lets say the box prefers height-for-width layouting, then
      first you get a nice minimum and natural size for both wrapping
      labels... it's possible at this point that the natural width
      of both labels are less than the full parent allocation limit
      in which case they will completely unwrap in the next step.

      While allocating the box in height for width, the minimum/natural
      allocation will have meaning and a shorter label will receive full
      width while the adjacent wrapping label might still wrap.

  c1) Of course the height is finally requested for the allocated child
      labels ... who have already been *naturally* with the minimum/natural
      allocation and the key point: Only enough height is reserved to fit
      the wrapping labels but no more.

In the end, you want your layout manager to prefer the request mode
preferred by the majority of your content, unless you have lots of
vertically rotated wrapping text actors, this choice is usually
height-for-width and rarely-to-never width-for-height.

Eeek my english suffers in a hurry.

I think you get the general drift though:

 "while the adjacent _longer_ label might still wrap."

 "who have already been _allocated_ *naturally* with the minimum/natural..."

Created attachment 220002 [details] [review]
Implements h4w requests in the opposing box orientation, patch revision 2

Fixed patch to comply to the review comments.

Created attachment 221409 [details] [review]
ClutterBoxLayout: Blessing with proper h4w geometry management

Just removed some trailing whitespaces, which triggered some warnings while testing it out.

Attachment 221409 [details] pushed as d037890 - ClutterBoxLayout: Blessing with proper h4w geometry management