Re: Bug in get_preferred_height_for_width() [was Re: Minimum height for minimum width]

[Tristan Van Berkom <tristanvb openismus com>]

On Fri, 2010-10-15 at 11:32 -0400, Havoc Pennington wrote:
> Hi,
> 
> I think I get what you're saying. If not I'll probably understand it
> reading your code.
> 
> btw things are looking kind of messed up to me in the current code in
> gtkwidget.c ... this:
> 
>       gtk_widget_get_preferred_width (widget, NULL, &natural_width);
>       get_span_inside_border_horizontal (widget,
> 					 aux_info,
> 					 allocation->width,
> 					 natural_width,
> 					 &x, &w);
> 
> so natural_width has the margins in it, right? But it's centered
> without removing those margins first. The code up in
> get_span_inside_border() removes margins from allocation->width but
> not natural_width.
> 
> It seems like we need to remove the margins to get
> adjusted_natural_width. And then say in GtkContainer, we need that
> adjusted_natural_width and we remove border_width from it, and then we
> pass the twice-adjusted natural width with both margins and border
> width and alignment-added-padding stripped down to the actual subclass
> like GtkButton.
> 
> Without trying to code it and see if it works, it could look like:
> 
> (* adjust_size_allocation) (GtkWidget *widget,
>                                        GtkOrientation orientation,
>                                        gint           *for_size_opposite,
>                                        gint           *natural_size,
>                                        gint           *offset,
>                                        gint           *adjusted_size);
> 
> where all four numbers are changing as we're chaining Widget->Container->Button
> 
> Might be nicer to do struct GtkAllocatedSize { int for_size_opposite;
> int natural_size; int offset; int adjusted_size }  ?


Ok my brain is about to explode trying to figure out the right
implementation for this, lets look in detail to the requirements.

>From what I understand, come time to adjust the size for allocation
GtkWidget needs to do the following steps (for simplicity, lets assume
GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH mode):

  - Strip any padding added by itself and any subclasses from the 
    allocation->width (this produces a 'stripped_allocated_width')

  - If halign != FILL, it needs to limit the width to the real natural
    size, this in itself involves:

      a.) calling gtk_widget_get_preferred_width()

      b.) stripping any padding from the returned natural width
	  (producing a 'stripped_natural_width')

      c.) interior width available for alignments becomes
          MIN (stripped_allocation_width, stripped_natural_width)

  - Now that we have the proper width for interior allocation; go ahead
    and strip any padding added to the allocation->height, i.e. get
    a 'stripped_allocated_height'.

  - If valign != FILL, we need to further limit the height to the
    proper natural height for width, the fun continues:

      a.) We need to *re-add* any required padding to the concluded
          interior allocation width, presumably by calling
          ->adjust_size_request() on our _final_ width which was
	  concluded by the previous steps.

      b.) We need to use this new adjusted width to obtain the natural
          height and do that by calling
          gtk_widget_get_preferred_height_for_width()

      c.) Now we need to strip the added padding from the returned
          natural height (getting us a usable
         'stripped_natural_height').

      d.) The interior height available for alignment/allocation
          now becomes:
          MIN (stripped_allocated_height, stripped_natural_height)

  - Now we have a proper width and height we can go on to align
    the interior allocation inside the padded space in both
    orientations.

Of course furthermore, gtk_widget_get_height_for_width needs to be
amended to adjust the for_width by:

  - Stripping any extra padding added by ->adjust_size_request from
    the for_width.

  - If halign != FILL then it needs to:

    a.) gtk_widget_get_preferred_width() to obtain the natural width
    b.) strip any padding previously added to the natural width
    c.) 'for_width' then becomes
        MIN (stripped_for_width, stripped_natural_width);

  - Call the class vfunc ->height_for_width() using the adjusted
    'for_width'

That's the big picture of "what needs to happen", however it's still
not mapped to any proper API... I've been tentatively writing some
pseudo code that should do it but I keep getting stuck somewhere.

Maybe splitting the adjust_size_allocation() and align_size_allocation()
makes sense, but I get the feeling that adding apis here is creating
a mess factor we dont want to deal with in the long run (and admittedly
I still dont have a clear picture of how it can all work).

There's also another alternative, all of this alignment/padding code
so far belongs to GtkWidget (and marginally GtkContainer), so does all
of the size-requesting logic... so we could go the direction of:

  - remove vfuncs ->adjust_size_allocation/->adjust_size_request

  - remove gtk_container_class_handle_border_width() and make it
    the default behaviour (by having GtkWidgetClass consider
    the container border width implicitly, all the time, or keep it
    as a flag for backwards compatability and still have GtkWidgetClass
    consider it implicitly if the flag is set).

  - Just "do the right thing" according to the steps above without
    the hassle of creating some API that will work when subclasses
    play heavily into size adjusting/aligning...

What are people's thoughts ?

> 
> Sorry I got this wrong in my original patch :-/
> 
> Havoc