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

[Tristan Van Berkom <tristanvb openismus com>]

On Sun, 2010-10-17 at 15:28 -0400, Havoc Pennington wrote:
> Hi,
> 
> I would think of it like this maybe
> 
> real_adjust_request(orientation, &request_data)
> {
>    adjust_request_by_adding_margin(orientation, &request_data)
>   /* alignment does not affect request */
> }
> 
> real_adjust_allocation(orientation, &allocation_data)
> {
>   adjust_allocation_by_removing_margin(orientation, &allocation_data)
>   adjust_allocation_by_aligning(orientation, &allocation_data)
> }
> 
> I think allocation_data *includes* the natural size, which is itself
> adjusted. So when you by_removing_margin you are dropping down the
> natural size you started with so that the align stuff has the
> marginless size. Same for for_size.
> 
> Conceptually GtkWidget contains two adjustments. But those two
> adjustments should not have anything to do with each other. It's as if
> you had a hierarchy like:
> 
> GtkWidget -> GtkWidgetWithMargin -> GtkWidgetWithAlignment
> 
> Those would chain up to each other. But since it's all in one class
> the two adjusts are invoked inline. However if our adjust vfunc has
> the right signature, it should be possible to do margin and alignment
> orthogonally.
> 
> On Sun, Oct 17, 2010 at 5:36 AM, Tristan Van Berkom
> <tristanvb openismus com> wrote:
> >  - Strip any padding added by itself and any subclasses from the
> >    allocation->width (this produces a 'stripped_allocated_width')
> 
> Widget base class need not strip what subclasses added - each subclass
> strips its own. When overriding, subclass has to chain up so widget
> can do its stuff just as container does.

You miss the big picture, this is not an implementation description
but a description of the problem as a whole.

So yes, GtkWidget *must* strip all paddings from GtkWidget and
GtkContainer, by invoking some method or something when 
gtk_widget_size_allocate() api is called.

> 
> I'd also say "margin" here for clarity rather than padding
> 
> >  - 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()
> >
> 
> I think get_preferred_width should be called outside of the adjust
> vfunc, and then initial natural size passed in. As the adjustment
> proceeds, the natural size is chopped down by each adjustment.
> 
> >      b.) stripping any padding from the returned natural width
> >          (producing a 'stripped_natural_width')
> 
> This should be done by the adjust_allocation_by_removing_margin() and
> the natural width that then gets passed for aligning would be reduced.
> 
> >      c.) interior width available for alignments becomes
> >          MIN (stripped_allocation_width, stripped_natural_width)
> 
> Hmm this doesn't sound right. Conceptually we have to decide if the
> margin is inside or outside the alignment-required padding. This
> basically means whether we do the margin adjust first or the align
> adjust first. (Note that on request you go from inner adjust to outer,
> i.e. chain up last, and on allocation the other direction, i.e. chain
> up first. So if margin is on the outside, it would be added second in
> request, and removed first in allocate.)

Is this not right ?

Widget margins and padding are removed unconditionally from the
allocated size.

Widget margins and padding are added to both minimum and natural
size requests (meaning at minimum, there is enough to strip an
allocation of its margins and padding.

So the widget's derived actual natural width, is indeed the
natural width from get_preferred_width() - margins/padding.

Isnt it ?

That decision furthermore (whether we are aligning at all), needs
to be taken into account by:
   adjust_allocation_by_aligning(orientation, &allocation_data);


> 

> 
> Anyway. What should come in to
> adjust_allocation_by_aligning(&allocation_data) should be a natural
> size de-margined and an allocation also de-margined. We just align the
> de-margined natural size in the de-margined allocation.
> 
> >  - 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'.
> 
> I think you go back and have your unadjusted width allocation, and you
> pass that as the for_width to the
> adjust_allocation(orientation=height). Now as the allocation
> adjustment proceeds, each adjust step has to also adjust for_width (in
> addition to the allocation itself and the natural size).
> 
> > 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.
> 
> Rather, each adjust_size_allocation "step" (align, margin,
> border_width are 3 "steps") should compensate for itself in the
> for_width as the chaining up proceeds.
> 
> > 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.
> 
> I think just adding the natural size, for_size, and orientation to the
> existing two vfuncs should work. Probably need a struct to hold {
> size, natural_size, for_size } which are the three things to adjust.
> 
> > 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
> 
> I think the adjust approach should be workable as described above and
> keep the nice encapsulation / flexibility of the vfuncs.

Maybe, I might give it one more shot, it might even by marginally 
workable but the question is also, how is it useful ?

Is it useful for any subclasses besides GtkContainer to add a
margin to the already marginable GtkWidget ?

For this to work for the GtkConainter case, already GtkContainer 
needs to be fixed to do its own work *first* and *then* chain up
to GtkWidgetClass, currently GtkContainer thinks its work
is independant of the whole mix, but goes ahead and removes
a border width from a possibly already realigned size.

What happens when another subclass wants to use
->adjust_size_allocation() to realign itself further ? how
can it cooperate with GtkWidgetClass and not cause bad side
effects ? ... and more importantly, why is this useful
to anybody except GtkWidget and GtkContainer ?

-Tristan

> 
> Havoc