GNOME Bugzilla – Bug 632766
Bug with height-for-width geometry management combined with adjust-size vfuncs.
Last modified: 2010-10-22 15:09:02 UTC
Created attachment 172895 [details] [review] Patch reworks adjust-size-request/allocation APIs and invokes it in the right places The bug is with gtk_widget_get_preferred_height_for_width() and gtk_widget_get_preferred_width_for_height() apis. They adjust the size request at output time correctly but fail to adjust the input for_size, this means that for aligned widgets (h/valign != FILL) the input 'for_size' is not adjusted to it's natural size, in some cases requesting less height than needed if given a larger than natural width. Also it was not removing margins, attached patch fixes the problem. As discussed in this thread: http://mail.gnome.org/archives/gtk-devel-list/2010-October/msg00113.html
Created attachment 172896 [details] [review] Updated test case This patch updates ./tests/testadjustsize to show the problem In the wrapping label test, I added an expanding label above so that the frame/wrapping-label below gets no extra space. Without applying the patch to fix, you will see that when horizontally aligning the frame/label to CENTER, everything works fine until the allocation is greater than natural size, after giving it enough horizontal size the label clips as it never requested a sufficient height to fit the aligned label.
Created attachment 172897 [details] [review] Fixed patch. The previous patch had problems. In gtkwidget.c:adjust_for_align() alignments were done by setting the proposed size = natural size, when infact it needs to *limit* the proposed size with the natural size. This updated patch fixes that (otherwise widgets dont request enough height when allocated less than natural width).
Created attachment 172898 [details] [review] Better test case This test case makes both the above label and below test frame/wrapping label expand. This better demonstrates the center/center alignment of the wrapping label below.
Review of attachment 172897 [details] [review]: This looks really good. Thanks for cleaning up my mess. ::: gtk/gtkwidget.h @@ +386,3 @@ void (* adjust_size_allocation) (GtkWidget *widget, + GtkOrientation orientation, + gint *natural_size, I was thinking, maybe we should pass in min size here as well, just for conceptual completeness - "when adjusting allocation, the entire size request is available" - since min size would not be computable inside an adjust_size_allocation. Since we're kinda getting a lot of args it's possible structifying this stuff makes sense... the logical structs would be one with the two request numbers and one with the two alloc numbers ?
Created attachment 172940 [details] [review] Even better patch This one is a bit more lightweight as it doesnt add ->adjust_size_request() inside gtk_widget_size_allocate() It also fixes some things in gtk_widget_real_adjust_size_allocation() that remove the need for the "white lie" and make everything a bit saner.
(In reply to comment #4) > Review of attachment 172897 [details] [review]: > > This looks really good. Thanks for cleaning up my mess. > > ::: gtk/gtkwidget.h > @@ +386,3 @@ > void (* adjust_size_allocation) (GtkWidget *widget, > + GtkOrientation orientation, > + gint *natural_size, > > I was thinking, maybe we should pass in min size here as well, just for > conceptual completeness - "when adjusting allocation, the entire size request > is available" - since min size would not be computable inside an > adjust_size_allocation. > > Since we're kinda getting a lot of args it's possible structifying this stuff > makes sense... the logical structs would be one with the two request numbers > and one with the two alloc numbers ? I would personally like to move on and stop thinking about this. However I agree adding min_size to the api is conceptually more complete, perhaps it could be useful if we wanted to force the widget to only allocate a proportion of its natural size (i.e. allocate 63% of the difference between minimum and natural ?). I dont think the arg list is getting too too long either in this case, I rather wonder if structifying the whole thing is running the risk of glorifying this api with good documentation (I would personally rather consider these vfuncs as a deep and dark secret of GTK+... I'm not sure what third party widget authors can achieve with this that they can't already achieve by using the widget alignment and margin properties already in place). Feel free to differ in opinion here, I just think GTK+ will be all around simpler if people don't think of implementing adjust_size_request/allocate() at all in their own widget implementations.
Landed patches in master now. Closing this bug... feel free to further enhance the api's if you feel it's appropriate/important.