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 660848 - window: Don't add window borders on startup
window: Don't add window borders on startup
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2011-10-04 01:28 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2011-10-18 01:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
window: Don't add window borders on startup (1.98 KB, patch)
2011-10-04 01:28 UTC, Jasper St. Pierre (not reading bugmail)
rejected Details | Review
Static testcase (521 bytes, text/x-python)
2011-10-17 18:03 UTC, Jasper St. Pierre (not reading bugmail)
  Details
Make meta_display_unmanage_screen public (1.77 KB, patch)
2011-10-17 19:27 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
Unmanage the screen before reexecing (994 bytes, patch)
2011-10-17 19:27 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
frame: Make sure to offset by invisible borders when unmanaging windows (2.03 KB, patch)
2011-10-17 21:20 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2011-10-04 01:28:50 UTC
Steps:

1. Position a window at the top-left
2. Restart gnome-shell
3. Notice that the window has moved down a bit. By the visible borders, exactly

This extra padding stuff is added by the math in adjust_for_gravity.

In meta_window_move_resize_internal, "Session restore" is marked as the same
case as a new window, but it doesn't look so. Maybe mutter isn't setting the
window position properly on exit?
Comment 1 Jasper St. Pierre (not reading bugmail) 2011-10-04 01:28:52 UTC
Created attachment 198172 [details] [review]
window: Don't add window borders on startup

https://bugzilla.gnome.org/show_bug.cgi?id=660839
Comment 2 Jasper St. Pierre (not reading bugmail) 2011-10-04 03:45:40 UTC
If I'm interpreting the ICCCM correctly, when we unmanage a window (transitions to Withdrawn), we should move it to where it would be if the frame didn't exist. We aren't doing that. I don't think we ever did that.
Comment 3 Dan Winship 2011-10-04 12:02:08 UTC
IIRC, getting windows to remain in the right place between window manager restarts is one of things that seems like it ought to be possible, but actually isn't. cc:ing Havoc since he should know why metacity behaves like it does now...
Comment 4 Havoc Pennington 2011-10-04 12:10:30 UTC
It should be possible; I used to restart metacity all the time while working on it without having windows march down the screen. On current metacity master it looks like meta_window_destroy_frame() is supposed to reparent to the right coordinates, but I don't know how it works out in mutter.
Comment 5 Jasper St. Pierre (not reading bugmail) 2011-10-04 18:50:23 UTC
It happens in metacity for me too.

  http://www.youtube.com/watch?v=xTVS-7bsFRQ

The testcase here is:

 $ curl http://magcius.mecheye.net/shell/static.py
 $ python static.py 0 # run with NW gravity

Running with static gravity, it all works fine.
Comment 6 Jasper St. Pierre (not reading bugmail) 2011-10-06 19:08:35 UTC
Apparently the static.py link isn't working (hosting provider is trying to run it as Python).

 $ curl http://magcius.mecheye.net/shell/static.txt > static.py
 $ python static.py 0 # run with NW gravity
Comment 7 Owen Taylor 2011-10-17 15:09:23 UTC
Test case is inaccessible (500) - always best to attach test cases, so they'll be around as long as the bug. The way that it seems to be intended to work in the metacity/mutter is that metacity fixes up the position on unmanage. meta_window_destroy_frame() used to have:

   XReparentWindow (window->display->xdisplay,
                    window->xwindow,
                    window->screen->xroot,
                   /* FIXME where to put it back depends on the gravity */
                    window->frame->rect.x,
                    window->frame->rect.y);

the FIXME was removed in 9ec6dbd5c (bug 399552) and replaced with the comment:

                   /* Using anything other than meta_window_get_position()
                    * coordinates here means we'll need to ensure a configure
                    * notify event is sent; see bug 399552.
                    */

But the FIXME isn't actually fixed, so the code seems right only for NW gravity windows. The reason that your test case isn't working for metacity is that you are killing metacity with Control-C so the above code doesn't run.

I don't think something along the lines of your patch can be fully right - the  correct adjustment depends on the _new_ window manager's frame dimensions - just leaving the client window in the same absolute place on the scree wouldn't work. So, I think we have to choose:

 A) Handle the clean shutdown case correctly
 B) Handle the unclean shutdown case correctly if we're being replaced by exactly the same window manager with the same theme.

I think A) is probably more important. And since this is the relevant code, it's clear what needs fixing to make invisible borders work with the clean shutdown - we need to reparent to the so the client window is where we'll adjust it back from on startup - the corner of the _visible_ border. Note that the code to send configure events in send_configure_notify() will need checking - read 9ec6dbd5c for inspiration.
Comment 8 Owen Taylor 2011-10-17 16:05:11 UTC
Review of attachment 198172 [details] [review]:

Marking rejected as (IMO) the wrong approach, as described in the previous comment.
Comment 9 Jasper St. Pierre (not reading bugmail) 2011-10-17 18:03:28 UTC
Created attachment 199237 [details]
Static testcase
Comment 10 Jasper St. Pierre (not reading bugmail) 2011-10-17 19:27:11 UTC
Created attachment 199255 [details] [review]
Make meta_display_unmanage_screen public
Comment 11 Jasper St. Pierre (not reading bugmail) 2011-10-17 19:27:28 UTC
Created attachment 199256 [details] [review]
Unmanage the screen before reexecing

This ensures a 'clean shutdown' of mutter.



--

This fixes the issue to me.
Comment 12 Jasper St. Pierre (not reading bugmail) 2011-10-17 21:20:02 UTC
Created attachment 199278 [details] [review]
frame: Make sure to offset by invisible borders when unmanaging windows

When we reparent a window to the root when we're exiting, we need to offset
the position by the invisible borders, otherwise windows will creep up and
to the left.



--

Sorry, I lied. We also have to do this, or windows will creep up and to the right.
Comment 13 Jasper St. Pierre (not reading bugmail) 2011-10-18 01:54:35 UTC
Attachment 199255 [details] pushed as f700a7b - Make meta_display_unmanage_screen public
Attachment 199278 [details] pushed as 6d0c1f0 - frame: Make sure to offset by invisible borders when unmanaging windows
Comment 14 Jasper St. Pierre (not reading bugmail) 2011-10-18 01:55:53 UTC
Attachment 199256 [details] pushed as 577ccc4 - Unmanage the screen before reexecing