Bug 1269437 - [GTK3][CSD on] gtk_window_resize() also counts client side decorations size
[GTK3][CSD on] gtk_window_resize() also counts client side decorations size
Status: CLOSED EOL
Product: Fedora
Classification: Fedora
Component: gtk3 (Show other bugs)
22
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Matthias Clasen
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-10-07 07:11 EDT by Martin Stransky
Modified: 2016-07-19 14:08 EDT (History)
11 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 1264355
: 1281817 (view as bug list)
Environment:
Last Closed: 2016-07-19 14:08:06 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
example (1.74 KB, text/x-csrc)
2015-10-07 07:11 EDT, Martin Stransky
no flags Details
Modified example (2.00 KB, text/plain)
2015-10-09 03:52 EDT, Olivier Fourdan
no flags Details


External Trackers
Tracker ID Priority Status Summary Last Updated
Mozilla Foundation 1209659 None None None Never

  None (edit)
Description Martin Stransky 2015-10-07 07:11:06 EDT
Created attachment 1080601 [details]
example

When CSD is enabled (export GTK_CSD=1) gtk_window_get_size()/gtk_window_resize() for toplevel windows also counts the shadow size.

So resized windows are smaller than expected and also actual window size is wrong.

See the attached example - actual window size is different when CSD is on and off.

This causes this upstream bug - https://bugzilla.mozilla.org/show_bug.cgi?id=1209659
Comment 1 Martin Stransky 2015-10-07 07:19:52 EDT
Is there any way how to get CSD shadow size? AFAIK there's gdk_window_set_shadow_width() to set the size but I don't see any counterpart. 

Can _NET_WM_OPAQUE_REGION query give an useful result here? (although it may not work on Wayland, right?)
Comment 2 Olivier Fourdan 2015-10-09 03:52 EDT
Created attachment 1081208 [details]
Modified example

I am not entirely sure about this bug.

gtk_window_resize() [1] does what it says, ie it resizes the window to the given size.

The problem is, when using CSD, part of that size is used for decorations and various handles (shadows, header bar, etc.) but these controls are part of the GtkWindow when CSD is used so that seems consistent to me.

Actually I think the way it works is that the GtkWindow is a container for other wiidgets so its actual size should be determined by its content, not the other way around.

So to get the size you want, I would set the size_request on the widget inside the window ("drawing" in your example) so that the actual window is sized accordingly.

I am attaching a modified version of your example to explain what I mean, that one works and gives the expected size.

I shall also discuss this issue further on irc #gtk+ to see what others reckons.

[1] https://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-resize
Comment 3 Martin Stransky 2015-10-09 09:23:16 EDT
There's a problem with gtk_widget_set_size_request() - the widget can't be resized under that values. So it can't replace the gtk_window_resize().
Comment 4 Matthias Clasen 2015-10-09 09:25:39 EDT
(In reply to Olivier Fourdan from comment #2)
> Created attachment 1081208 [details]
> Modified example
> 
> I am not entirely sure about this bug.
> 
> gtk_window_resize() [1] does what it says, ie it resizes the window to the
> given size.
> 
> The problem is, when using CSD, part of that size is used for decorations
> and various handles (shadows, header bar, etc.) but these controls are part
> of the GtkWindow when CSD is used so that seems consistent to me.

I do consider this a bug, but it is one that is very hard to fix.

It would be much better if applications did not care about the exact size of their toplevels. The only real reason to care is if you directly draw on the toplevel, which is another thing that is best avoided.
Comment 5 Martin Stransky 2015-10-09 09:29:09 EDT
(In reply to Matthias Clasen from comment #4)
> It would be much better if applications did not care about the exact size of
> their toplevels. The only real reason to care is if you directly draw on the
> toplevel, which is another thing that is best avoided.

In Firefox in CSD (and Wayland) mode we don't draw to top-level widget any more. We draw to the drawing area inside the toplevel window. But we still need to set exact size of the drawing area and thus of the top-level window and get the move/resize events here. How we can do that?
Comment 6 Martin Stransky 2015-10-09 09:33:51 EDT
Right now I try to get the extents by:

gtk_window_get_size(GTK_WINDOW(mShell), &width, &height);
gdk_window_get_frame_extents(GDK_WINDOW(window), &rect);

mExtentWidth = rect.width - width;
mExtentHeight = rect.height - height;

and increase the sire request of the top-level window this those extents. But not luck yet, maybe I do something wrong here.
Comment 7 Olivier Fourdan 2015-10-09 10:12:32 EDT
(In reply to Martin Stransky from comment #3)
> There's a problem with gtk_widget_set_size_request() - the widget can't be
> resized under that values. So it can't replace the gtk_window_resize().

Yes, indeed, it doesn't replace gtk_window_resize().

That would be the minimal size of the window for its content to fit - Unless you want the user to be able to shrink the window more than it should to keep its content visible.

(In reply to Martin Stransky from comment #6)
> Right now I try to get the extents by:
> 
> gtk_window_get_size(GTK_WINDOW(mShell), &width, &height);
> gdk_window_get_frame_extents(GDK_WINDOW(window), &rect);
> 
> mExtentWidth = rect.width - width;
> mExtentHeight = rect.height - height;
> 
> and increase the sire request of the top-level window this those extents.
> But not luck yet, maybe I do something wrong here.

gdk_window_get_frame_extents() [1] gives "the bounding box of the window, including window manager titlebar/borders if any".

So on X11 [2] it relies on the WM to set _NET_FRAME_EXTENTS [3][4] or walk the window tree to find the Window manager's own window [5].

But with CSD, the window manager will not decorate the client window, so that's unlikely to work. The client alone knows its decoration size, by definition.

And "_GTK_FRAME_EXTENTS" won't help either as it accounts for the shadows alone (not counting the header bar).

[1] https://git.gnome.org/browse/gtk+/tree/gdk/gdkwindow.c?h=gtk-3-18#n10145
[2] https://git.gnome.org/browse/gtk+/tree/gdk/x11/gdkwindow-x11.c?h=gtk-3-18#n3117
[3] http://standards.freedesktop.org/wm-spec/wm-spec-1.3.html#idm140130317541248
[4] https://git.gnome.org/browse/gtk+/tree/gdk/x11/gdkwindow-x11.c?h=gtk-3-18#n3171
[5] https://git.gnome.org/browse/gtk+/tree/gdk/x11/gdkwindow-x11.c?h=gtk-3-18#n3214
Comment 8 Martin Stransky 2015-10-12 02:28:35 EDT
Okay, Thanks for the explanation. So is there any way how to get the CSD size from GTK?
Comment 9 Olivier Fourdan 2015-10-12 04:05:32 EDT
(In reply to Martin Stransky from comment #8)
> Okay, Thanks for the explanation. So is there any way how to get the CSD
> size from GTK?

Not that I can think of myself, but it doesn't mean there is none (maybe someone else more knowledgeable could figure some other way).
Comment 10 Martin Stransky 2015-10-12 06:46:14 EDT
I tried some ways like:

- get size of outer (toplevel) window and the inner (drawing) window and compare the sizes (by gdk_window_get_geometry)
- get the child allocation (by gtk_widget_get_allocation)

but not luck yet. The inner (drawing) window is always returned as 1x1 size. Mya be related to mozilla custom widget implementation. still investigating.
Comment 11 Martin Stransky 2015-10-12 12:48:12 EDT
Guys, I find this workaround, but I need to make sure it's a correct one:

+        GtkAllocation shellAllocation = {0,0,1000,1000};
+        GtkAllocation contAllocation;
+
+        gtk_widget_size_allocate(GTK_WIDGET(toplevel_widget), &shellAllocation);
+        gtk_widget_get_allocation(GTK_WIDGET(drawing_widget), &contAllocation);
+
+        shadow_width = (shellAllocation.width - contAllocation.width);
+        shadow_height = (shellAllocation.height - contAllocation.height);

I just request allocation of size for the child widget on the toplevel and read the real allocated size from child (if I understand correctly what's going on here). 

IMHO it's a bug - I expect the child allocation is really requested 1000x1000 but it's shrank by CSD shadow size.

Thoughts?
Comment 12 Olivier Fourdan 2015-10-13 02:57:38 EDT
(In reply to Martin Stransky from comment #11)
> Guys, I find this workaround, but I need to make sure it's a correct one:
> 
> [...]
> I just request allocation of size for the child widget on the toplevel and
> read the real allocated size from child (if I understand correctly what's
> going on here). 

That should work, indeed, at least this matches what actually happens - gtk_window_resize() will resize the window, but with CSD the actual size of the content is reduced by the shadows and head bar.

> IMHO it's a bug - I expect the child allocation is really requested
> 1000x1000 but it's shrank by CSD shadow size.
> 
> Thoughts?

Yes, that's what Matthias said in comment 4, it's a bug (but not that easy to fix)
Comment 13 Matthias Clasen 2015-10-13 10:40:14 EDT
(In reply to Martin Stransky from comment #11)
> Guys, I find this workaround, but I need to make sure it's a correct one:
> 
> +        GtkAllocation shellAllocation = {0,0,1000,1000};
> +        GtkAllocation contAllocation;
> +
> +        gtk_widget_size_allocate(GTK_WIDGET(toplevel_widget),
> &shellAllocation);
> +        gtk_widget_get_allocation(GTK_WIDGET(drawing_widget),
> &contAllocation);
> +
> +        shadow_width = (shellAllocation.width - contAllocation.width);
> +        shadow_height = (shellAllocation.height - contAllocation.height);
> 
> I just request allocation of size for the child widget on the toplevel and
> read the real allocated size from child (if I understand correctly what's
> going on here). 
> 

Calling gtk_widget_get_allocation at random times is not guaranteed to work or give expected results. Note that the shadow size can change depending on a number of factors, e.g. theme changes, maximization, etc...

So, while it might work now, there's tons of corner cases where this can and likely will break. I would look really hard at getting away from the need to know this.
Comment 14 Martin Stransky 2015-10-14 04:42:58 EDT
(In reply to Matthias Clasen from comment #13)
> Calling gtk_widget_get_allocation at random times is not guaranteed to work
> or give expected results. Note that the shadow size can change depending on
> a number of factors, e.g. theme changes, maximization, etc...
> 
> So, while it might work now, there's tons of corner cases where this can and
> likely will break. I would look really hard at getting away from the need to
> know this.

Well, and is there any other way how to get the extents? It's a blocker bug for Firefox - all dialog windows are smaller than expected, there are missing/invisible buttons and so on. For instance the shadow extents are ~100 pixels wide on Fedora 22 - you even can't close the browser ;-)
Comment 15 Olivier Fourdan 2015-10-15 09:11:47 EDT
I've filed a bug upstream to track this and attached a patch there, probably wrong in many respects, but this is mostly to get the ball rolling.

It still fails with your reproducer but that's a good demonstration of the limitations of such an approach.

What the reproducer does is:

  window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
  gtk_window_resize(window,500,500);
  ...
  gtk_widget_show_all (GTK_WIDGET(window));
  gtk_window_get_size(GTK_WINDOW(window), &width, &height);
  gtk_window_resize(GTK_WINDOW(window), width, height);

The problem is that when the first resize is performed, the window is not realized, therefore the CSD title bar does not exist, so it's impossible to add up its height in the size computation.

Then the window is shown, thus realized, all the widgetry gets created, so when get_size gets called, now the CSD title bar exists and is visible, so GTK+ with the proposed patch will rightfully deduce the height of the title bar and therefore you will end up with a slightly smaller window than expected.

So with the proposed patch in GTK+, the reproducer would need to set the requested size after the window is shown.

I am still looking into improving the logic.
Comment 16 Martin Stransky 2015-10-15 10:43:52 EDT
I'm happy with a solution when resize/get_size is called after realize but before show.
Comment 17 Olivier Fourdan 2015-10-19 03:34:44 EDT
(In reply to Martin Stransky from comment #16)
> I'm happy with a solution when resize/get_size is called after realize but
> before show.

Hi Martin, before I do a slight rework with comments and try to move forward with the patch upstream in gtk+, could you try it and make sure it does help fixing the issue with Firefox?

Patch is attached here: https://bugzilla.gnome.org/show_bug.cgi?id=756618
Comment 18 Martin Stransky 2015-10-19 03:51:24 EDT
Sure. Shall I apply the patch to gtk package (is gtk3-3.16.7 enough?) and rebuild FF with that?
Comment 19 Olivier Fourdan 2015-10-19 04:13:56 EDT
(In reply to Martin Stransky from comment #18)
> Sure. Shall I apply the patch to gtk package (is gtk3-3.16.7 enough?) and
> rebuild FF with that?

The patch is for current git master, but it can be cherry-picked as-is in the branch for gtk-3-16 so, yes, it should apply on gtk3-3.16.7 just fine.

As for Firefox, I haven't looked at the code but what I wrote in comment 15 still stands, i.e. that patch in gtk+ will work better if the gtk_window_resize() is invoked after the widgets are realized, something along these lines:

    /* create a new window */
    window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
    gtk_widget_realize (GTK_WIDGET (window));
    gtk_window_resize (window, 500, 500);
    gtk_widget_show_all (GTK_WIDGET (window));
Comment 20 Martin Stransky 2015-10-19 08:57:19 EDT
The patch seems to work, without any Firefox changes. Tested on Fedora 22 + the patch and latest Firefox trunk + CSD enabled.
Comment 21 Olivier Fourdan 2015-10-19 09:22:41 EDT
Nice!

On F23 using your github repo on Wayland, the profile window is still not tall enough, due to the CSD title bar not being accounted for so I suspect FF still does resize then realize/show afterwards.

I'll see if I can come up with a patch (but FF is a huge codebase...)
Comment 22 Olivier Fourdan 2015-10-19 10:05:32 EDT
(In reply to Olivier Fourdan from comment #21)
> I'll see if I can come up with a patch (but FF is a huge codebase...)

I patched widget/gtk/nsWindow.cpp to issue a gtk_widget_realize() prior to resizing the window but that did not make much difference.

But then I realized (no pun intended of course!) that Firefox does not seem to work well with CSD at all, it looks like it draws on the toplevel window directly so that the CSD shadows disappear and the title bar appears misplaced within the window.

That goes far beyond the window size issue it seems so I cannot conclude and I'll leave to you to decide if that patch helps or not (it seems, according to comment 20)
Comment 23 Martin Stransky 2015-10-20 03:08:47 EDT
(In reply to Olivier Fourdan from comment #21)
> Nice!
> 
> On F23 using your github repo on Wayland, the profile window is still not
> tall enough, due to the CSD title bar not being accounted for so I suspect
> FF still does resize then realize/show afterwards.

That Wayland repo is really a WIP of wayland patch and is not updated. New trunk contains fixes for CSD (https://bugzilla.mozilla.org/show_bug.cgi?id=1195002, https://bugzilla.mozilla.org/show_bug.cgi?id=1210249) and works fine with your patch.

The primary goal here is to make CSD work with current Gtk3 on X11 to allow Mozilla ship Gtk3 by default so don't worry about Wayland now.
Comment 24 Olivier Fourdan 2015-10-20 08:05:07 EDT
Ah, thanks, I get a tad confused with all the current Firefox/gtk3 related bugs... What matters most is that the patch helps, though, so all is well :-)
Comment 25 Olivier Fourdan 2015-10-28 12:30:29 EDT
FYI, the patches have been pushed upstream.
Comment 26 Fedora End Of Life 2016-07-19 14:08:06 EDT
Fedora 22 changed to end-of-life (EOL) status on 2016-07-19. Fedora 22 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.

Note You need to log in before you can comment on or make changes to this bug.