Bug 1269437
Summary: | [GTK3][CSD on] gtk_window_resize() also counts client side decorations size | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Martin Stransky <stransky> | ||||||
Component: | gtk3 | Assignee: | Matthias Clasen <mclasen> | ||||||
Status: | CLOSED EOL | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||
Severity: | unspecified | Docs Contact: | |||||||
Priority: | unspecified | ||||||||
Version: | 22 | CC: | alexl, anton4linux, ccecchi, extras-qa, gecko-bugs-nobody, jadahl, johannespfrang, jorgeml, mclasen, ofourdan, stransky | ||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Whiteboard: | |||||||||
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 18:08:06 UTC | Type: | Bug | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Attachments: |
|
Description
Martin Stransky
2015-10-07 11:11:06 UTC
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?) 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 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(). (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. (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? 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. (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 Okay, Thanks for the explanation. So is there any way how to get the CSD size from GTK? (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). 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. 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? (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) (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. (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 ;-) 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. I'm happy with a solution when resize/get_size is called after realize but before show. (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 Sure. Shall I apply the patch to gtk package (is gtk3-3.16.7 enough?) and rebuild FF with that? (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)); The patch seems to work, without any Firefox changes. Tested on Fedora 22 + the patch and latest Firefox trunk + CSD enabled. 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...) (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) (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. 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 :-) FYI, the patches have been pushed upstream. 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. |