On Thu, 2010-10-21 at 14:10 +0900, Tristan Van Berkom wrote: > On Thu, 2010-10-21 at 13:57 +0900, Tristan Van Berkom wrote: > > 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 > > > <trista...@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: > > > > Actually even that is wrong, since we're passing a size that > has it's margins stripped to get_height_for_width(), which > will do the operation again on the for_size. > > So it needs to add another line: > > > --------------------------------------------------- > > > > 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); > > After adjusting adjusting the allocated width with the aid of the real > natural width... we get a new adjusted_allocation.width, before passing > that width to get_preferred_height_for_width we need to: > > gint adjusted_allocated_width = adjusted_allocation.width; > > GTK_WIDGET_GET_CLASS (widget)->adjust_size_request(widget, > GTK_ORIENTATION_HORIZONTAL, > /* for_size is never needed > for this api afaics */ > &adjusted_allocated_width, > NULL); > > And then we need to call get_height_for_width () to obtain the natural > height for the allocated width... /after/ adding some margins to the > inner allocated width, since those margins will be removed from the > for_size deep inside get_height_for_width() and then others re-added > to the output natural height. > > Thanks for trying a patch out with this new API (it helps alot), > I'll try updating the patch to do what I think it needs to do and > send that one back. > > I'll remove the for_size argument from ->adjust_size_request() as > well since it's not used anywhere and I cant imagine what it can > be used for. >
I've attached my patch here, with your API approach seems everything falls into place. Besides the changes I outlined above I also had to change GtkContainer adjust_size_allocation() implementation to do it's own magic *first* and then chain up to GtkWidgetClass after. Otherwise with extra-large allocations, a container border width would be *further removed* aligned allocations. This should be the one. Cheers, -Tristan > Cheers, > -Tristan > > > > > 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 > > > > > > _______________________________________________ > > gtk-devel-list mailing list > > gtk-devel-list@gnome.org > > http://mail.gnome.org/mailman/listinfo/gtk-devel-list > > > _______________________________________________ > gtk-devel-list mailing list > gtk-devel-list@gnome.org > http://mail.gnome.org/mailman/listinfo/gtk-devel-list
diff --git a/gtk/gtkcontainer.c b/gtk/gtkcontainer.c index f80b633..2470d4e 100644 --- a/gtk/gtkcontainer.c +++ b/gtk/gtkcontainer.c @@ -323,11 +323,13 @@ static void gtk_container_map (GtkWidget *widget); static void gtk_container_unmap (GtkWidget *widget); static void gtk_container_adjust_size_request (GtkWidget *widget, GtkOrientation orientation, - gint for_size, gint *minimum_size, gint *natural_size); static void gtk_container_adjust_size_allocation (GtkWidget *widget, - GtkAllocation *allocation); + GtkOrientation orientation, + gint *natural_size, + gint *allocated_pos, + gint *allocated_size); static gchar* gtk_container_child_default_composite_name (GtkContainer *container, GtkWidget *child); @@ -1776,7 +1778,6 @@ gtk_container_resize_children (GtkContainer *container) static void gtk_container_adjust_size_request (GtkWidget *widget, GtkOrientation orientation, - gint for_size, gint *minimum_size, gint *natural_size) { @@ -1797,28 +1798,33 @@ gtk_container_adjust_size_request (GtkWidget *widget, /* chain up last so gtk_widget_set_size_request() values * will have a chance to overwrite our border width. */ - parent_class->adjust_size_request (widget, orientation, for_size, + parent_class->adjust_size_request (widget, orientation, minimum_size, natural_size); } static void gtk_container_adjust_size_allocation (GtkWidget *widget, - GtkAllocation *allocation) + GtkOrientation orientation, + gint *natural_size, + gint *allocated_pos, + gint *allocated_size) { GtkContainer *container; int border_width; container = GTK_CONTAINER (widget); - parent_class->adjust_size_allocation (widget, allocation); - if (!GTK_CONTAINER_GET_CLASS (widget)->handle_border_width) - return; + { + parent_class->adjust_size_allocation (widget, orientation, + natural_size, allocated_pos, + allocated_size); + return; + } border_width = container->priv->border_width; - allocation->width -= border_width * 2; - allocation->height -= border_width * 2; + *allocated_size -= border_width * 2; /* If we get a pathological too-small allocation to hold * even the border width, leave all allocation to the actual @@ -1828,23 +1834,26 @@ gtk_container_adjust_size_allocation (GtkWidget *widget, * As long as we have space, set x,y properly. */ - if (allocation->width < 1) + if (*allocated_size < 1) { - allocation->width += border_width * 2; + *allocated_size += border_width * 2; } else { - allocation->x += border_width; + *allocated_pos += border_width; + *natural_size -= border_width * 2; } - if (allocation->height < 1) - { - allocation->height += border_width * 2; - } - else - { - allocation->y += border_width; - } + /* Chain up to GtkWidgetClass *after* removing our border width from + * the proposed allocation size. This is because it's possible that the + * widget was allocated more space than it needs in a said orientation, + * if GtkWidgetClass does any alignments and thus limits the size to the + * natural size... then we need that to be done *after* removing any margins + * and padding values. + */ + parent_class->adjust_size_allocation (widget, orientation, + natural_size, allocated_pos, + allocated_size); } /** diff --git a/gtk/gtksizerequest.c b/gtk/gtksizerequest.c index 55979f4..da91d0c 100644 --- a/gtk/gtksizerequest.c +++ b/gtk/gtksizerequest.c @@ -226,20 +226,56 @@ compute_size_for_orientation (GtkWidget *request, requisition_size = requisition.width; if (for_size < 0) - GTK_WIDGET_GET_CLASS (request)->get_preferred_width (request, &min_size, &nat_size); + { + GTK_WIDGET_GET_CLASS (request)->get_preferred_width (request, &min_size, &nat_size); + } else - GTK_WIDGET_GET_CLASS (request)->get_preferred_width_for_height (request, for_size, - &min_size, &nat_size); + { + int ignored_position = 0; + int natural_height; + + /* Pull the base natural height from the cache as it's needed to adjust + * the proposed 'for_size' */ + gtk_widget_get_preferred_height (widget, NULL, &natural_height); + + /* convert for_size to unadjusted height (for_size is a proposed allocation) */ + GTK_WIDGET_GET_CLASS (request)->adjust_size_allocation (widget, + GTK_ORIENTATION_VERTICAL, + &natural_height, + &ignored_position, + &for_size); + + GTK_WIDGET_GET_CLASS (request)->get_preferred_width_for_height (request, for_size, + &min_size, &nat_size); + } } else { requisition_size = requisition.height; if (for_size < 0) - GTK_WIDGET_GET_CLASS (request)->get_preferred_height (request, &min_size, &nat_size); + { + GTK_WIDGET_GET_CLASS (request)->get_preferred_height (request, &min_size, &nat_size); + } else - GTK_WIDGET_GET_CLASS (request)->get_preferred_height_for_width (request, for_size, - &min_size, &nat_size); + { + int ignored_position = 0; + int natural_width; + + /* Pull the base natural width from the cache as it's needed to adjust + * the proposed 'for_size' */ + gtk_widget_get_preferred_width (widget, NULL, &natural_width); + + /* convert for_size to unadjusted width (for_size is a proposed allocation) */ + GTK_WIDGET_GET_CLASS (request)->adjust_size_allocation (widget, + GTK_ORIENTATION_HORIZONTAL, + &natural_width, + &ignored_position, + &for_size); + + GTK_WIDGET_GET_CLASS (request)->get_preferred_height_for_width (request, for_size, + &min_size, &nat_size); + } } pop_recursion_check (request, orientation); @@ -270,7 +306,6 @@ compute_size_for_orientation (GtkWidget *request, orientation == GTK_SIZE_GROUP_HORIZONTAL ? GTK_ORIENTATION_HORIZONTAL : GTK_ORIENTATION_VERTICAL, - cached_size->for_size, &adjusted_min, &adjusted_natural); diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c index 474f55a..1f62e24 100644 --- a/gtk/gtkwidget.c +++ b/gtk/gtkwidget.c @@ -572,11 +572,13 @@ static void gtk_widget_queue_tooltip_query (GtkWidget *widg static void gtk_widget_real_adjust_size_request (GtkWidget *widget, GtkOrientation orientation, - gint for_size, gint *minimum_size, gint *natural_size); static void gtk_widget_real_adjust_size_allocation (GtkWidget *widget, - GtkAllocation *allocation); + GtkOrientation orientation, + gint *natural_size, + gint *allocated_pos, + gint *allocated_size); static void gtk_widget_set_usize_internal (GtkWidget *widget, gint width, @@ -4496,6 +4498,8 @@ gtk_widget_size_allocate (GtkWidget *widget, gboolean alloc_needed; gboolean size_changed; gboolean position_changed; + gint natural_width, natural_height; + gint min_width, min_height; priv = widget->priv; @@ -4532,7 +4536,82 @@ gtk_widget_size_allocate (GtkWidget *widget, real_allocation = *allocation; adjusted_allocation = real_allocation; - GTK_WIDGET_GET_CLASS (widget)->adjust_size_allocation (widget, &adjusted_allocation); + if (gtk_widget_get_request_mode (widget) == GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH) + { + gint adjusted_for_width, unused; + + 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); + + /* Reduce the allocated width by it's margins and possibly limit it + * to the natural width */ + GTK_WIDGET_GET_CLASS (widget)->adjust_size_allocation (widget, + GTK_ORIENTATION_HORIZONTAL, + &natural_width, + &adjusted_allocation.x, + &adjusted_allocation.width); + + /* Add the margins to the contextual for_size for get_height_for_width */ + unused = adjusted_for_width = adjusted_allocation.width; + GTK_WIDGET_GET_CLASS (widget)->adjust_size_request (widget, + GTK_ORIENTATION_HORIZONTAL, + &unused, + &adjusted_for_width); + + /* Get the natural height for the real allocated width (including margins) */ + gtk_widget_get_preferred_height_for_width (widget, adjusted_for_width, NULL, &natural_height); + + /* Now go ahead and remove any margins from the height and possibly limit the height to + * the natural height */ + GTK_WIDGET_GET_CLASS (widget)->adjust_size_allocation (widget, + GTK_ORIENTATION_VERTICAL, + &natural_height, + &adjusted_allocation.y, + &adjusted_allocation.height); + } + else + { + gint adjusted_for_height, unused; + + gtk_widget_get_preferred_height (widget, &min_height, &natural_height); + + /* "natural_width" is a white lie; it's reduced to the + * actual allocation but kept above min. + */ + if (natural_height > real_allocation.height) + natural_height = MAX(real_allocation.height, min_height); + + /* Reduce the allocated height by it's margins and possibly limit it + * to the natural height */ + GTK_WIDGET_GET_CLASS (widget)->adjust_size_allocation (widget, + GTK_ORIENTATION_VERTICAL, + &natural_height, + &adjusted_allocation.y, + &adjusted_allocation.height); + + /* Add the margins to the contextual for_size for get_width_for_height */ + unused = adjusted_for_height = adjusted_allocation.height; + GTK_WIDGET_GET_CLASS (widget)->adjust_size_request (widget, + GTK_ORIENTATION_VERTICAL, + &unused, + &adjusted_for_height); + + /* Get the natural width for the real allocated height (including margins) */ + gtk_widget_get_preferred_width_for_height (widget, adjusted_for_height, NULL, &natural_width); + + /* Now go ahead and remove any margins from the width and possibly limit the width to + * the natural width */ + GTK_WIDGET_GET_CLASS (widget)->adjust_size_allocation (widget, + GTK_ORIENTATION_HORIZONTAL, + &natural_width, + &adjusted_allocation.x, + &adjusted_allocation.width); + } if (adjusted_allocation.x < real_allocation.x || adjusted_allocation.y < real_allocation.y || @@ -4807,140 +4886,70 @@ gtk_widget_real_size_allocate (GtkWidget *widget, } static void -get_span_inside_border (GtkWidget *widget, - GtkAlign align, - int start_pad, - int end_pad, - int allocated_outside_size, - int natural_inside_size, - int *coord_inside_p, - int *size_inside_p) +adjust_for_align(GtkAlign align, + gint *natural_size, + gint *allocated_pos, + gint *allocated_size) { - int inside_allocated; - int content_size; - int coord, size; - - inside_allocated = allocated_outside_size - start_pad - end_pad; - - content_size = natural_inside_size; - if (content_size > inside_allocated) - { - /* didn't get full natural size */ - content_size = inside_allocated; - } - - coord = size = 0; /* silence compiler */ switch (align) { case GTK_ALIGN_FILL: - coord = start_pad; - size = inside_allocated; + /* change nothing */ break; case GTK_ALIGN_START: - coord = start_pad; - size = content_size; + /* keep *allocated_pos where it is */ + *allocated_size = *natural_size; break; case GTK_ALIGN_END: - coord = allocated_outside_size - end_pad - content_size; - size = content_size; + *allocated_pos += (*allocated_size - *natural_size); + *allocated_size = *natural_size; break; case GTK_ALIGN_CENTER: - coord = start_pad + (inside_allocated - content_size) / 2; - size = content_size; + *allocated_pos += (*allocated_size - *natural_size) / 2; + *allocated_size = *natural_size; break; } - - if (coord_inside_p) - *coord_inside_p = coord; - - if (size_inside_p) - *size_inside_p = size; -} - -static void -get_span_inside_border_horizontal (GtkWidget *widget, - const GtkWidgetAuxInfo *aux_info, - int allocated_outside_width, - int natural_inside_width, - int *x_inside_p, - int *width_inside_p) -{ - get_span_inside_border (widget, - aux_info->halign, - aux_info->margin.left, - aux_info->margin.right, - allocated_outside_width, - natural_inside_width, - x_inside_p, - width_inside_p); } static void -get_span_inside_border_vertical (GtkWidget *widget, - const GtkWidgetAuxInfo *aux_info, - int allocated_outside_height, - int natural_inside_height, - int *y_inside_p, - int *height_inside_p) -{ - get_span_inside_border (widget, - aux_info->valign, - aux_info->margin.top, - aux_info->margin.bottom, - allocated_outside_height, - natural_inside_height, - y_inside_p, - height_inside_p); +adjust_for_margin(gint start_margin, + gint end_margin, + gint *natural_size, + gint *allocated_pos, + gint *allocated_size) +{ + *natural_size -= (start_margin + end_margin); + *allocated_pos += start_margin; + *allocated_size -= (start_margin + end_margin); } static void gtk_widget_real_adjust_size_allocation (GtkWidget *widget, - GtkAllocation *allocation) + GtkOrientation orientation, + gint *natural_size, + gint *allocated_pos, + gint *allocated_size) { const GtkWidgetAuxInfo *aux_info; - gint natural_width; - gint natural_height; - int x, y, w, h; aux_info = _gtk_widget_get_aux_info_or_defaults (widget); - if (gtk_widget_get_request_mode (widget) == GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH) + if (orientation == GTK_ORIENTATION_HORIZONTAL) { - gtk_widget_get_preferred_width (widget, NULL, &natural_width); - get_span_inside_border_horizontal (widget, - aux_info, - allocation->width, - natural_width, - &x, &w); - - gtk_widget_get_preferred_height_for_width (widget, w, NULL, &natural_height); - get_span_inside_border_vertical (widget, - aux_info, - allocation->height, - natural_height, - &y, &h); + adjust_for_margin (aux_info->margin.left, + aux_info->margin.right, + natural_size, allocated_pos, allocated_size); + adjust_for_align (aux_info->halign, + natural_size, allocated_pos, allocated_size); } - else /* GTK_SIZE_REQUEST_WIDTH_FOR_HEIGHT */ + else { - gtk_widget_get_preferred_height (widget, NULL, &natural_height); - get_span_inside_border_vertical (widget, - aux_info, - allocation->height, - natural_height, - &y, &h); - - gtk_widget_get_preferred_width_for_height (widget, h, NULL, &natural_width); - get_span_inside_border_horizontal (widget, - aux_info, - allocation->width, - natural_width, - &x, &w); + adjust_for_margin (aux_info->margin.top, + aux_info->margin.bottom, + natural_size, allocated_pos, allocated_size); + adjust_for_align (aux_info->valign, + natural_size, allocated_pos, allocated_size); } - - allocation->x += x; - allocation->y += y; - allocation->width = w; - allocation->height = h; } static gboolean @@ -9897,7 +9906,6 @@ gtk_widget_real_size_request (GtkWidget *widget, static void gtk_widget_real_adjust_size_request (GtkWidget *widget, GtkOrientation orientation, - gint for_size, gint *minimum_size, gint *natural_size) { diff --git a/gtk/gtkwidget.h b/gtk/gtkwidget.h index c7faeec..08f58b7 100644 --- a/gtk/gtkwidget.h +++ b/gtk/gtkwidget.h @@ -161,7 +161,10 @@ struct _GtkWidget * and alignment properties of #GtkWidget. Chain up * <emphasis>before</emphasis> performing your own adjustments so your * own adjustments remove more allocation after the #GtkWidget base - * class has already removed margin and alignment. + * class has already removed margin and alignment. The natural size + * passed in should be adjusted in the same way as the allocated size, + * which allows adjustments to perform alignments or other changes + * based on natural size. */ struct _GtkWidgetClass { @@ -378,11 +381,13 @@ struct _GtkWidgetClass void (* adjust_size_request) (GtkWidget *widget, GtkOrientation orientation, - gint for_size, gint *minimum_size, gint *natural_size); void (* adjust_size_allocation) (GtkWidget *widget, - GtkAllocation *allocation); + GtkOrientation orientation, + gint *natural_size, + gint *allocated_pos, + gint *allocated_size); /*< private >*/
_______________________________________________ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list