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

[Tristan Van Berkom <tristanvb openismus com>]

On Thu, 2010-10-14 at 10:45 -0400, Havoc Pennington wrote:
> In sounds like in short the for_size somehow needs to be adjusted
> (strip out the pixels that adjust_size_request added). (If I'm
> understanding properly.) Of course that's what adjust_size_allocation
> does, except it's both dimensions. for_size is after all the proposed
> allocation size in one dimension.

Ouch, indeed.

The for_size fed to widget implementations additionally needs to
strip the added padding that can happen in ->adjust_size_request()

However the bug I'm referring to is another one; the for_size also
needs to be limited to the natural size in the case that the widget
does not "fill" (i.e. not to compensate for pixels stripped in
->adjust_size_request(), but for pixels that will further be stripped
in ->adjust_size_allocation(), in the case that the widget expanded).

> Maybe just change adjust_size_allocation to take a GtkOrientation and
> return a single integer (could have a convenience function to do the
> current full GtkAllocation adjust). Then use that to clean up for_size
> when it comes in to compute_size_for_orientation() in
> gtksizerequest.c. Or I think for now it'd also work to make a
> GtkAllocation allocation = { MAXINT, for_height } and adjust that,
> seems less hacky to change adjust_size_allocation to be able to do
> just one dimension.

Right, this will be a bit more difficult as the current
adjust_size_allocation() needs to use the context of both
APIs (i.e. it needs to call get_request_mode() and then
either get_preferred_width and get_preferred_height_for_width()
or the other way around).

However changing that API seems like the right thing to do,
I suppose it will need to take the natural_width as an argument,
which seems strange.. the api would look similar to the current
gtkwidget.c static functions:
 get_span_inside_border_horizontal/vertical().

In the end the adjust_size_allocation() api/vfunc 
will look like this:

void gtk_widget_adjust_allocated_size (GtkWidget *widget,
				       GtkOrientation orientation,
				       gint           proposed_size,
				       gint           natural_size,
				       gint          *offset,
				       gint          *adjusted_size);

And then gtk_widget_adjust_allocation() would be a convenience API
that would call the above api, this convenience API would function
in the same way as the current implementation does internally:
   'gtk_widget_real_adjust_size_allocation()'

The part that feels weird here is that we are feeding in the
natural_size, however it's important because come allocation time
specifically; the 'natural_size' may be in context to a for_size
in the other orientation (i.e. the widget cannot be expected to
just "know" its natural size in this stage).

I'll give this approach a shot with the above API change and
file a bug for it shortly...

-Tristan

PS: After looking at this, it feels also a bit strange that we need
to have ->adjust_size_request/allocation() (i.e. it seems that the
added border-width that GtkContainer computes here should just be 
pulled in by chaining up vfuncs through parent classes during
the request/allocate cycles), however I suppose there's nothing
to be done for that... or is there...

> I don't think a solution that looks at xalign specifically is needed,
> so we can keep the abstraction barrier. The generic mechanism just
> needs to be smart enough to know that the for_size is a proposed
> (adjusted up) allocation and thus has to be unadjusted before
> computing the unadjusted request.
> 
> Havoc