GNOME Bugzilla – Bug 751341
GtkWindow: fix default empty window size with CSD
Last modified: 2015-07-02 15:13:53 UTC
This tries to fix some problems uncovered in bug 750343
Created attachment 305852 [details] [review] GtkWindow: some min/nat size corrections. Don't add the container border to the title request size; it is only used for the child widget. Don't call gtk_widget_get_preferred_width_for_height() for the titlebar with an unrelated height. ---------- This is not strictly needed, but I noticed while working on the real fix.
Created attachment 305853 [details] [review] GtkWindow: fix default empty window size with CSD In the non-CSD case we checked for 0x0 window size requisition and replaced it with 200x200 so the window was still visible. This no longer works in case of CSD as the shadow and title bar are always added to the requisition resulting in a titlebar/shadow only window in case there is no child widget (this is currently visible under wayland or when setting GTK_CSD=1). Instead of special casing the final window size, special case the child requisition paths instead. This gives us the same requisition in both, CSD and non-CSD cases (the header bar has a too large minimum width atm so the resulting window is still not the same)
Created attachment 305900 [details] [review] GtkHeaderBar: reduce minimum title width The minimum title width affects the minimum window width for CSD windows. To allow smaller windows like without CSD reduce it a bit (276px vs 156px min width)
Review of attachment 305852 [details] [review]: ::: gtk/gtkwindow.c @@ +8443,2 @@ + title_min += window_border.left + window_border.right; + title_nat += window_border.left + window_border.right; While I agree there's a problem here, I don't think this is entirely correct either. It needs to be something like title_height = get_preferred_height (title) get_preferred_width_for_height (title, title_height, &title_min, &title_nat) get_preferred_width_for_height (child, height - title_height, &child_min, &child_nat) min = MAX (title_min, child_min) bat = NAX (title_nat, child_nat)
Review of attachment 305853 [details] [review]: I can't say that I find this corner case super-important, but the patch looks right.
Review of attachment 305900 [details] [review]: ok
Created attachment 305913 [details] [review] GtkWindow: some min/nat size corrections. Thanks. I've noticed that the child request width/height could get negative in there when passing in a too small height/width. Added some checks for that as well.
Review of attachment 305913 [details] [review]: Looks good now.
Thanks for the quick review!
Christoph, I bisected reftests broken on master to this patch: GtkWindow: fix default empty window size with CSD. Could you help fixing the issue?
I'll have a look tomorrow.
Created attachment 306282 [details] [review] GtkWindow: don't increase the preferred size for empty windows if there is a size request set. This fixes a reftest broken by commit 84e99b20ac806ee5f --------- I now get the same test failures as before my commit.
Review of attachment 306282 [details] [review]: ::: gtk/gtkwindow.c @@ +8101,3 @@ + gint width, height; + + gtk_widget_get_size_request (GTK_WIDGET (window), &width, &height); I would use _gtk_widget_get_aux_info_or_defaults() here, instead: static gboolean window_has_size_request (GtkWidget *window) { const GtkWidgetAuxInfo *info = _gtk_widget_get_aux_info_or_defaults (window); return !(info->width == -1 && info->height == -1); } Which is what gtk_widget_get_size_request() does, anyway, but it saves us a type check. We could even have a private _gtk_widget_has_size_request() as well. @@ +8407,3 @@ gtk_widget_get_preferred_width (child, &child_min, &child_nat); + if (child_nat == 0 && !_gtk_window_has_size_request (window)) Instead of calling this multiple times you should probably call it once at the top of each function, and store the return value.
Created attachment 306321 [details] [review] GtkWindow: don't increase the preferred size for empty windows if there is a size request set. Thanks. I went with a private _gtk_widget_has_size_request()
Review of attachment 306321 [details] [review]: ::: gtk/gtkwidget.c @@ +11100,3 @@ } /** This should be: /*< private > Otherwise gtk-doc will think it's a public symbol (yes: _gtk_widget_override_size_request below is also wrong). Also, you don't need the leading '_': if a symbol is not explicitly marked as exported, it is considered private.
Created attachment 306344 [details] [review] GtkWindow: don't increase the preferred size for empty windows if there is a size request set.
Review of attachment 306344 [details] [review]: Would be good to say in the commit message which reftest it fixes. Looks good otherwise.
Pushed this to unbreak reftests. Attachment 306344 [details] pushed as 5d17b0a - GtkWindow: don't increase the preferred size for empty windows if there is a size request set.
Since Cosimo was faster, this fixed background-position-simple, css-match-class, css-match-exact, css-match-name, css-match-subtype, css-match-type, gtk-icontheme-sizing, window-default-size