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 762713 - [Wayland] GDK doesn't remember unmaximized or unfullscreen size
[Wayland] GDK doesn't remember unmaximized or unfullscreen size
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
3.19.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-02-26 08:10 UTC by Jonas Ådahl
Modified: 2016-03-01 12:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Simple reproducer for testing purpose. (1.44 KB, text/plain)
2016-02-26 10:23 UTC, Olivier Fourdan
  Details
wayland: restore size when configure size is 0x0 (5.28 KB, patch)
2016-02-26 12:42 UTC, Olivier Fourdan
none Details | Review
[PATCH v2] wayland: restore size when configure size is 0x0 (5.27 KB, patch)
2016-02-26 13:04 UTC, Olivier Fourdan
none Details | Review
[PATCH v3] wayland: restore size when configure size is 0x0 (4.80 KB, patch)
2016-02-26 13:06 UTC, Olivier Fourdan
none Details | Review
Simple reproducer for testing purpose. (2.49 KB, text/plain)
2016-02-29 09:05 UTC, Olivier Fourdan
  Details
[PATCH v4] wayland: restore size when configure size is 0x0 (5.34 KB, patch)
2016-02-29 09:07 UTC, Olivier Fourdan
committed Details | Review

Description Jonas Ådahl 2016-02-26 08:10:27 UTC
According to xdg_shell, a xdg_surface.configure with size 0x0 should be interpreted as that it is up to the client to set a size. When being unfullscreened this should mean the client should configure its size to what it was before being maximized or fullscreened.

This problem currently only reproduces on weston, since weston will send a configure with size 0x0 when unmaximizing or unfulscreening.

How to reproduce:

1. Open weston.
2. Run for example gnome-terminal in that weston session.
3. Press F11 to fullscreen
4. Press F11 to unfullscreen

The size size after 4 is now the same as after 3, except the window now has window decorations.

I tried a bit to make gdk/wayland remember the window size and configure the size properly when the compositor sends 0x0, but it seems the meaning of window->size is very context dependent, and remembering that either resulted in window growing incorrectly or shrinking incorrectly, depending on whether the shadow width was taken into account.

How should window->width/height be interpreted? Because right now it seems that for a non-resizing window, it'll switch between including shadow width and excluding, without the actual window changing size.
Comment 1 Olivier Fourdan 2016-02-26 08:25:38 UTC
The problem might be because either fullscreen and maximized windows don't have shadows whereas normal state windows normally have shadows, and with CSD, shadows are part of the toplevel gdkwindow size, so that makes such transitions a tad more tricky.

I need to look further into this, I'll give it a try.
Comment 2 Jonas Ådahl 2016-02-26 08:33:02 UTC
(In reply to Olivier Fourdan from comment #1)
> The problem might be because either fullscreen and maximized windows don't
> have shadows whereas normal state windows normally have shadows, and with
> CSD, shadows are part of the toplevel gdkwindow size, so that makes such
> transitions a tad more tricky.
> 
> I need to look further into this, I'll give it a try.

Not so sure about that. I added a function "gdk_wayland_window_remember_size()" which only would remember if the window state was not fullscreen and not maximized. The size difference of width/height I saw the unfullscreen/unmaximized sizes (around 400-500, where fullscreen were around 1024x768) +- ~50 or something (shadow width I'm assuming).
Comment 3 Olivier Fourdan 2016-02-26 10:23:18 UTC
Created attachment 322447 [details]
Simple reproducer for testing purpose.

Attaching a simple reproducer that I used for another bug.

You need to take into account the margins that change when fullscreen and maximized.

Something like that in your gdk_wayland_window_remember_size() function:

  impl->saved_width = window->width - impl->margin_left - impl->margin_right;
  impl->saved_height = window->height - impl->margin_top - impl->margin_bottom;

That fixes the issue in my case.

I reckon that's because gtk+ does change its shadow width prior to change states (can be observed by breaking on gdk_wayland_window_set_shadow_width).

That brings us to another issue, which is as to when to save sizes because gtk+ also resize the window to something as large as the screen prior to change its state... So the saved sizes are wrong.
Comment 4 Olivier Fourdan 2016-02-26 12:42:20 UTC
Created attachment 322457 [details] [review]
wayland: restore size when configure size is 0x0

According to xdg_shell, an xdg_surface.configure with size 0x0 should
be interpreted as that it is up to the client to set a size.

When transitioning from maximize or fullscreen state, this means the
client should configure its size back to what it was before being
maximize or fullscreen.

This problem currently only occurs on weston because weston sends a
configure with size 0x0 when transitioning back from maximize or
fullscreen.
Comment 5 Olivier Fourdan 2016-02-26 13:04:03 UTC
Created attachment 322459 [details] [review]
[PATCH v2] wayland: restore size when configure size is 0x0

Updated patch:

 - Reword comment
 - save size unconditionally in xdg_surface_configure()
Comment 6 Olivier Fourdan 2016-02-26 13:06:31 UTC
Created attachment 322461 [details] [review]
[PATCH v3] wayland: restore size when configure size is 0x0

Sorry, previous patch removed a blank line by mistake.
Comment 7 Jonas Ådahl 2016-02-27 16:17:26 UTC
Review of attachment 322461 [details] [review]:

Tried the patch and it works as advertised. I tried my test cases which were failing with my attempt and they are working as expected.

The only issue I can think of is if the window is resized while being hidden, after having previously been visible and ever maximized or fullscreened before. If so, we'll use the size saved from before it was resized instead of the size it was resized to..
Comment 8 Olivier Fourdan 2016-02-29 09:05:40 UTC
Created attachment 322631 [details]
Simple reproducer for testing purpose.

(In reply to Jonas Ådahl from comment #7)
> The only issue I can think of is if the window is resized while being
> hidden, after having previously been visible and ever maximized or
> fullscreened before. If so, we'll use the size saved from before it was
> resized instead of the size it was resized to..

Yeap, well spotted, can be easily reproduced with the attached reproducer.
Comment 9 Olivier Fourdan 2016-02-29 09:07:27 UTC
Created attachment 322632 [details] [review]
[PATCH v4] wayland: restore size when configure size is 0x0

Updated patch:
 - v4: Clear saved size when hiding the Wayland surface
Comment 10 Matthias Clasen 2016-02-29 23:34:35 UTC
Review of attachment 322632 [details] [review]:

::: gdk/wayland/gdkwindow-wayland.c
@@ +2335,2 @@
   if (impl->display_server.xdg_surface)
     xdg_surface_set_maximized (impl->display_server.xdg_surface);

Do we really need to save in maximize and fullscreen ? Shouldn't it be enough to save whenever we configure the surface with a new size ?

But you probably do need an extra save() call in sync_margin(), since the saved size depends on the margins ?
Comment 11 Olivier Fourdan 2016-03-01 07:51:26 UTC
(In reply to Matthias Clasen from comment #10)
> Review of attachment 322632 [details] [review] [review]:
> 
> ::: gdk/wayland/gdkwindow-wayland.c
> @@ +2335,2 @@
>    if (impl->display_server.xdg_surface)
>      xdg_surface_set_maximized (impl->display_server.xdg_surface);
> 
> Do we really need to save in maximize and fullscreen ? Shouldn't it be
> enough to save whenever we configure the surface with a new size ?

That was my first approach as well but that won't work because gtk+ reconfigures the window to a much larger size prior to changing its state (see comment 3), so by saving the size in the configure function, we'd save the wrong size.

> But you probably do need an extra save() call in sync_margin(), since the
> saved size depends on the margins ?

We save the size only when not fullscreen nor maximized, and we reuse them when leaving that state (and only when the given size is 0x0), so even if the margins do change while fullscreen or maximized, or even when in normal state we don't really care, we're not using/applying them.

But more importantly, saving the size in sync_margin() does break the case Jonas brought up in comment 7, ie a resize while hidden so I think it's better as it is now.
Comment 12 Matthias Clasen 2016-03-01 11:31:40 UTC
Review of attachment 322632 [details] [review]:

ok, lets go with this then
Comment 13 Olivier Fourdan 2016-03-01 12:18:06 UTC
Comment on attachment 322632 [details] [review]
[PATCH v4] wayland: restore size when configure size is 0x0

attachment 322632 [details] [review] pushed as git commit 3607b9a wayland: Restore size when configure size is 0x0