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

[Tristan Van Berkom <tristanvb openismus com>]

On Mon, 2010-10-18 at 01:28 -0400, Havoc Pennington wrote:
> Hi,
> 
> On Sun, Oct 17, 2010 at 11:58 PM, Tristan Van Berkom
> <tristanvb openismus com> wrote:
> >
> > 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 ?
> 
> In the patch I posted (assuming the FIXME is fixed), what would still be broken?
> I'm sort of lost what problems are unsolved. Granted, I might find out
> if I tested the patch ;-)
> 

This part will be re-broken, in this patch you are not adjusting the
allocated width for margins and alignments *before* getting the height
for the real allocated width:

---------------------------------------------------
  adjusted_allocation = real_allocation;
  if (gtk_widget_get_request_mode (widget) ==
      GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH)
    {
      int min_width;

      gtk_widget_get_preferred_width (widget, &min_width,
                                      &natural_width);
      /* "natural_width" is a white lie; it's reduced to the
       * actual allocation but kept above min.
       */
      if (natural_width > real_allocation.width)
        natural_width = MAX(real_allocation.width, min_width);
      gtk_widget_get_preferred_height_for_width (widget, natural_width,
                                                 NULL, &natural_height);
    }
  else ...
---------------------------------------------------

And then it goes on too adjust the allocations after checking
the height for width:
---------------------------------------------------

  GTK_WIDGET_GET_CLASS (widget)->adjust_size_allocation (widget,
                                         GTK_ORIENTATION_HORIZONTAL,
                                         &natural_width,
                                         &adjusted_allocation.x,
                                         &adjusted_allocation.width);
  GTK_WIDGET_GET_CLASS (widget)->adjust_size_allocation (widget,
                                         GTK_ORIENTATION_VERTICAL,
                                         &natural_height,
                                         &adjusted_allocation.y,
                                         &adjusted_allocation.height);
---------------------------------------------------

Let's see what we can do with this api... it would have to be
something more like:

---------------------------------------------------

if (widget_is_height_for_width) {

  gtk_widget_get_preferred_width (widget, &min_width,
                                  &natural_width);
  GTK_WIDGET_GET_CLASS (widget)->adjust_size_allocation (widget,
                                         GTK_ORIENTATION_HORIZONTAL,
                                         &natural_width,
                                         &adjusted_allocation.x,
                                         &adjusted_allocation.width);

  gtk_widget_get_preferred_height_for_width (widget, 
                                             adjusted_allocation.width,
                                             NULL, &natural_height);
  GTK_WIDGET_GET_CLASS (widget)->adjust_size_allocation (widget,
                                         GTK_ORIENTATION_VERTICAL,
                                         &natural_height,
                                         &adjusted_allocation.y,
                                         &adjusted_allocation.height);
}

---------------------------------------------------

>From first glance at the portion that touches 
gtksizerequest.c, the answer to the FIXME comment is
pretty much: yes it's going to be in the cache, we need
to call gtk_widget_get_preferred_width() there directly
to get a "probably cached" natural_width that has already been
treated by ->adjust_size_request().


> > ... and more importantly, why is this useful
> > to anybody except GtkWidget and GtkContainer ?
> 
> It is useful in any abstract base class that wants to provide "stuff
> around" whatever its subclasses draw.
> 
> I think GtkContainer is actually a good enough reason to have this.
> border-width is deprecated sure, but it's not going away soon, it'd be
> nice to clean up all the code that has to deal with it.
> 
> Another example in GTK is GtkMisc, though we want to deprecate that
> too, you could use this vfunc to delete the align and pad handling
> from its subclasses and delete some code, which would be nice.
> 
> Hypothetically you could do things like:
>  * a base class that aligned in a more precise way than
> left/right/center (like GtkAlignment)
>  * a base class providing more complex CSS-like border/margin/pad
> capability with colors for each
>  * a base class that provided a frame
>  * a base class that adds any kind of display or status next to subclass content
> 
> All of these could also be implemented as a container, granted. (That
> is, GtkMisc and GtkAlignment solve the same problem.)
> However, I think there can be good reasons to do this stuff in a base
> class so your widgets can have the stuff "built in"

Hmm GtkMisc is another case that has to be handled I had overlooked
that, possibly the GtkMisc classes have to implement the vfunc without
ever chaining up to GtkWidgetClass (possibly doing something forceful
by manually observing the GtkWidgetClass's "margins" to take them into
account and then doing it's own custom alignment logic, thus ignoring
the v/halign properties).


> 
> Havoc