After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 751341 - GtkWindow: fix default empty window size with CSD
GtkWindow: fix default empty window size with CSD
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
3.17.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-06-22 19:13 UTC by Christoph Reiter (lazka)
Modified: 2015-07-02 15:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GtkWindow: some min/nat size corrections. (2.13 KB, patch)
2015-06-22 19:15 UTC, Christoph Reiter (lazka)
none Details | Review
GtkWindow: fix default empty window size with CSD (4.65 KB, patch)
2015-06-22 19:16 UTC, Christoph Reiter (lazka)
committed Details | Review
GtkHeaderBar: reduce minimum title width (743 bytes, patch)
2015-06-23 09:00 UTC, Christoph Reiter (lazka)
committed Details | Review
GtkWindow: some min/nat size corrections. (4.16 KB, patch)
2015-06-23 11:48 UTC, Christoph Reiter (lazka)
committed Details | Review
GtkWindow: don't increase the preferred size for empty windows if there is a size request set. (3.53 KB, patch)
2015-06-29 11:51 UTC, Christoph Reiter (lazka)
none Details | Review
GtkWindow: don't increase the preferred size for empty windows if there is a size request set. (5.95 KB, patch)
2015-06-29 19:42 UTC, Christoph Reiter (lazka)
none Details | Review
GtkWindow: don't increase the preferred size for empty windows if there is a size request set. (5.95 KB, patch)
2015-06-29 21:54 UTC, Christoph Reiter (lazka)
accepted-commit_now Details | Review

Description Christoph Reiter (lazka) 2015-06-22 19:13:57 UTC
This tries to fix some problems uncovered in bug 750343
Comment 1 Christoph Reiter (lazka) 2015-06-22 19:15:36 UTC
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.
Comment 2 Christoph Reiter (lazka) 2015-06-22 19:16:12 UTC
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)
Comment 3 Christoph Reiter (lazka) 2015-06-23 09:00:46 UTC
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)
Comment 4 Matthias Clasen 2015-06-23 11:09:18 UTC
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)
Comment 5 Matthias Clasen 2015-06-23 11:10:30 UTC
Review of attachment 305853 [details] [review]:

I can't say that I find this corner case super-important, but the patch looks right.
Comment 6 Matthias Clasen 2015-06-23 11:12:37 UTC
Review of attachment 305900 [details] [review]:

ok
Comment 7 Christoph Reiter (lazka) 2015-06-23 11:48:07 UTC
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.
Comment 8 Matthias Clasen 2015-06-24 00:35:49 UTC
Review of attachment 305913 [details] [review]:

Looks good now.
Comment 9 Christoph Reiter (lazka) 2015-06-24 09:17:01 UTC
Thanks for the quick review!
Comment 10 Cosimo Cecchi 2015-06-28 18:24:54 UTC
Christoph, I bisected reftests broken on master to this patch:
GtkWindow: fix default empty window size with CSD.

Could you help fixing the issue?
Comment 11 Christoph Reiter (lazka) 2015-06-28 21:22:30 UTC
I'll have a look tomorrow.
Comment 12 Christoph Reiter (lazka) 2015-06-29 11:51:04 UTC
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.
Comment 13 Emmanuele Bassi (:ebassi) 2015-06-29 12:30:37 UTC
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.
Comment 14 Christoph Reiter (lazka) 2015-06-29 19:42:52 UTC
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()
Comment 15 Emmanuele Bassi (:ebassi) 2015-06-29 21:33:18 UTC
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.
Comment 16 Christoph Reiter (lazka) 2015-06-29 21:54:07 UTC
Created attachment 306344 [details] [review]
GtkWindow: don't increase the preferred size for empty windows if there is a size request set.
Comment 17 Matthias Clasen 2015-07-01 15:01:10 UTC
Review of attachment 306344 [details] [review]:

Would be good to say in the commit message which reftest it fixes. Looks good otherwise.
Comment 18 Cosimo Cecchi 2015-07-01 16:34:33 UTC
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.
Comment 19 Christoph Reiter (lazka) 2015-07-02 15:13:53 UTC
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