GNOME Bugzilla – Bug 628828
Add padding and alignment properties to GtkWidget
Last modified: 2010-09-19 05:18:50 UTC
This patch series first adds adjust_size_request, adjust_size_allocation virtual methods to GtkWidget, then uses those to implement existing aux info, then to implement GtkContainer::border-width (with opt-in, GtkButton is ported as an example), and finally to add padding and alignment to aux info. I am leaning toward renaming padding-left,right,top,bottom to margin-left,right,top,bottom and padding-all-sizes to simply "margin" But I'll let people discuss names before I do the search-and-replace.
Created attachment 169509 [details] [review] add _gtk_widget_get_aux_info_or_defaults() This is better than peeking aux info then testing != NULL in several ways: - it returns const aux info so if we don't create we can't write - it ensures that the default we assume if aux_info is NULL is the same as the default we set if we've created the aux info - it avoids typing in != NULL checks
Created attachment 169510 [details] [review] Use _gtk_widget_get_aux_info_or_defaults() when possible in gtkwidget.c Did not update uses in other files because the plan is to get rid of those other uses anyhow. So don't want to make this function available in the header.
Created attachment 169511 [details] [review] GtkWidget: add adjust_size_request adjust_size_allocation virtual funcs Use these new methods to handle set_size_request (aka aux_info) inside gtkwidget.c, instead of having external code mess with it. The virtual functions can be used for other purposes in the future. For example, GtkContainer::border_width could be automatically implemented for all container subclasses.
Created attachment 169512 [details] [review] Add testadjustsize test, to test new adjust size methods and related features This will test size adjust, and interactions with other padding and border properties.
Created attachment 169513 [details] [review] add gtk_container_class_handle_border_width() so subclasses can ignore border_width A subclass calls gtk_container_class_handle_border_width() in its class_init This marks the subclass as expecting GtkContainer to deal with border width automatically, which GtkContainer then does.
Created attachment 169514 [details] [review] GtkButton: let GtkContainer handle border width
Created attachment 169515 [details] [review] Add padding and alignment properties to GtkWidget h-align = START,END,CENTER,FILL v-align = START,END,CENTER,FILL padding-left,right,top,bottom,all-sides These should obsolete all such similar properties on layout containers, GtkMisc, GtkAlignment, GtkContainer::border-width
Created attachment 169516 [details] [review] Add padding and alignment tests to testadjustsize.c
bug 628829 fix is required for this series to work.
Created attachment 169537 [details] [review] Make set_size_request override container border width (patch to be squashed) When chaining up to adjust_size_request in GtkWidget base class, GtkWidget has to go last so the values from set_size_request can force the request back to what was set. With the previous code, border_width was added on top of set_size_request values.
Created attachment 169538 [details] [review] call gtk_widget_set_redraw_on_allocate() on alignment that we paint to (to be squashed) This patch should be squashed. The alignment in the test program didn't paint properly because it didn't queue redraw on allocate.
Created attachment 169539 [details] [review] document that widget padding is outside set_size_request (to be squashed) This patch should be squashed If padding were not outside the set_size_request() it would not work the same way as container-supplied (child property) padding. Conceptually set_size_request() forces the value from the subclass (the original unadjusted request) and then we go on to adjust the request further by adding the padding.
It would be instructive to see how a container (say GtkBox) uses the new align properties instead of its ::fill child property. In this new world, does ::expand stay a child property ?
GtkBox doesn't have to do anything, of course; it's just that now its fill and padding properties are redundant. So if you were inventing GtkBox from scratch you'd want to omit fill and padding. Since we already have GtkBox, those props are there - and still work. In fact, I guess you have to set the box's fill property to TRUE to use widget alignment in the box's orientation, because otherwise the child won't get extra space and the child's align won't come into effect. Perpendicular to the box, GtkBox always has fill=true behavior, so the perpendicular align prop on Widget would work without doing anything special. With this patch, the ideal GtkBox built from scratch would not have a fill child property (but would always do fill=true); would still have a child property for expand; and would not have a child property for padding. A side note, I defaulted h-align and v-align to FILL, not CENTER, while GtkBox defaults to fill=false which is like CENTER. h-align and v-align really have to default to FILL for backward compat; otherwise setting fill=true on a GtkBox child would not work anymore. Without compat concerns, maybe CENTER would be preferred, since it seems GtkBox switched to fill=false at some point. But, I think defaulting to CENTER would break a _lot_ of stuff and is not realistic. expand is unaffected by this series of patches. I saw a note in the wiki about a "spread" flag that Rapicorn has, which is like expand except it propagates up through all containers. I haven't looked, but my imaginary theory on this feature is that it's basically like a get_h_expand()/get_v_expand() virtual method on Widget, where for containers if any child has set expand=true, the container is also expand=true unless it's been explicitly set false. Then layout containers would look at get_h_expand/get_v_expand on their children, rather than having their own child property for it. And at the top level, Window would be resizable if it had this expand flag set. Anyway this sounds like a nice idea, but it is a separate (though perhaps related) problem, not touched in this patch. With just this patch, containers still need child properties for expand. Widget::h-align and ::v-align are just about what a child does with extra space (whether the child leaves it in the allocation or takes it out), they don't affect the allocation that comes from the parent container.
Ok, that makes sense. So, I guess with this way of doing things, we will deprecate child properties like GtkBox::fill GtkBox::padding GTK_FILL in GtkTable::x/y-options GtkTable::x/y-padding GTK_WRAP_BOX_H/V_FILL in GtkWrapBox::packing GtkWrapBox::horizontal/vertical-padding and aim for removing the whole lot in 4.0 (together with GtkMisc and GtkAlignment). Do you think it makes sense to look at baseline alignment in this context ?
Review of attachment 169515 [details] [review]: ::: gtk/gtkwidget.h @@ +568,3 @@ + * Just fix GtkBorder itself? The main danger is probably going signed + * to unsigned, rather than 32 to 16. + */ I think we should just fix GtkBorder. Just doing that test-wise seems to work fine. Probably just need to adjust gtk_rc_property_parse_border.
btw I'll look at an expand flag on Widget, started playing with that a bit already. will make a new bug once there's anything to show. I'm not sure how baseline stuff works or would work. I guess if the allocation contained a baseline somehow, then GTK_ALIGN_ON_BASELINE, ABOVE_BASELINE, BELOW_BASELINE, CENTER_ON_BASELINE, could work. GTK_ALIGN_ON_BASELINE would require getting the baseline of the child, so widgets would need get_baseline_for_height or something. If your allocation has a baseline and you know the child's baseline, you could line up the child baseline with allocation baseline. get_baseline_for_height probably also covers the needs of size request, so a "line up everything on the baseline" container would be possible. It seems like baseline layout is complex enough that it's going to require adding some new interfaces and a new container, so maybe it should just be kept separate from the non-baseline layout. Or maybe it's impossible to do non-holistically and GtkSizeRequest needs get_baseline_for_height and GtkAllocation needs a baseline field. I'm really not sure. (GTK has no existing baseline-layout features, right?) I do agree that designers enjoy specifying things in terms of baseline and that it would be nice to support. Surely baseline support can be a separate patch set from this, it may modify how this stuff works, but it would also modify how the previous stuff worked.
(In reply to comment #17) > > It seems like baseline layout is complex enough that it's going to require > adding some new interfaces and a new container, so maybe it should just be kept > separate from the non-baseline layout. Or maybe it's impossible to do > non-holistically and GtkSizeRequest needs get_baseline_for_height and > GtkAllocation needs a baseline field. I'm really not sure. (GTK has no existing > baseline-layout features, right?) The original height-for-width SoC had baseline features in its scope, but that didn't really work out, and Mathias Hasselmann focused more on the hfw part. > Surely baseline support can be a separate patch set from this, it may modify > how this stuff works, but it would also modify how the previous stuff worked. Yeah
Could someone explain how the API for drawing the widget is supposed to look? I assume widgets don't have to care about borders, padding, etc anymore?
See the patch to GtkButton https://bugzilla.gnome.org/attachment.cgi?id=169514&action=diff If you call gtk_container_class_handle_border_width() then your allocation has border pre-removed so you can just ignore its existence. However you have to switch each widget over to this mode, as in that patch. I only did GtkButton so far. The same thing could be done for GtkMisc and its subclasses, if desired, but I haven't done that yet. The new padding and alignment properties on GtkWidget are different since there's no back compat issue; they are always pre-handled - you can ignore them when drawing.