GNOME Bugzilla – Bug 111902
Windows maximized on placement not handled well
Last modified: 2004-12-22 21:47:04 UTC
We have a couple of issues in HEAD with placement in the case of maximized windows: 1) We try to get a per-xinerama work area for the window in meta_window_new, before it is placed, to determine whether we should maximize it automatically. 2) When maximized windows are mapped, no attempt is made to place them intelligently; currently they're always placed onto the first xinerama. What we need to do to fix this (proposal): 1) Move the auto-maximize heuristic out of window_new and into perhaps meta_window_contrain or meta_window_place (the problem with moving it to meta_window_place is that you can't call meta_window_maximize from there and have a reasonable hope of it actually working) 2) We should only try to automaximize if find_first_fit placement fails; i.e. it's possible that though the window is too large for one work area, there is another with an empty work area that would accomodate it. 3) If find_first_fit fails, we should traverse the natural xinerama ordering to see if we can find an empty xinerama to which we could move the window, and then maximize it onto that xinerama. We may want to add caching to the get_natural_xinerama list to improve performance with possible multiple calls to it. 4) If there is no empty xinerama, and the window is too large for the current xinerama, maximize the window to the current xinerama (the one with the pointer). 5) If a window starts out maximized, we should do a "partial placement" that would consist of simply looking for an empty xinerama where the window could reside. Unfortunately a lot about how window placement is done makes it tough to deal with this case, so I wanted to solicit suggestions before I try to do any serious coding on this.
Your proposal sounds about right to me, though as you say there are lots of details that might be tricky. If meta_window_maximize() were discouraged from synchronously doing a move resize, it seems like it'd be OK to call it from meta_window_place(). Dunno. This reminds me that the history patch is still pending...
*** Bug 105270 has been marked as a duplicate of this bug. ***
*** Bug 103318 has been marked as a duplicate of this bug. ***
I'm attaching a patch that implements the above policy. I made some changes to the way placement works that gets rid of any use of the "current" xinerama before placement, as well as handling maximized and unmaximized windows according to the policy outlined above. I've also added a meta_window_maximize_manual to be called from inside the constraints code when needed, and a window->should_maximize variable to indicate basically that a hint has been set that the window should maximize itself. This way the window will be placed and then maximized. This is a change in semantics from before, but I don't think that it will break anything and makes this code much much more sane. Also, I've gotten rid of the function contrain_placement, which was horribly broken before anyway and does not appear to be needed anymore because: 1) the placement code will never try to place a window off screen 2) it ignored struts anyway 3) it was using the "current" xinerama 4) it really just duplicated functionality already provided by the onscreen_constraints Note that there is a slight merge required for this to work with the other patch I posted today, but it's easy and won't be a problem. Hopefully we can get both patches in quickly (hint hint) so we don't need to worry about that ;-). -Rob
Created attachment 17309 [details] [review] handle maximized windows and work areas much better
marking High. Wow, I wrote two patches for the two biggest things I wanted to fix most in metacity. Time to start working on world peace and cold fusion...
You're kicking ass this weekend. All I have on this one is nitpick questions that occurred to me while reading the patch: - without constrain_placement(), won't the cascade algorithm place windows with only a little corner visible? Or did you make it smarter than that earlier? - I'd say should_maximize should be named something slightly more descriptive; perhaps "maximize_after_placement" describes it? (if so, it seems natural to me to do the maximizing at the end of meta_window_place()) - maximize_manual() could also use a better name I think; also, shouldn't meta_window_maximize() somehow be implemented by calling maximize_manual()? - do you happen to know why I put meta_window_raise() in meta_window_maximize()? ;-)
constrain_placement: no, it doesn't. That was actually one of the first patches I wrote for metacity -- making cascade smarter. maximize_after_placement: done maximize_manual: I renamed it to meta_window_maximize_internal and changed regular maximize to call it. I had intended to do that before but I guess I forgot to. I don't know if that name is any better, but at least it ought to encourage people to not call it directly. raise: my guess is sloppy/mouse focus + keyboard shortcuts. It would seem weird to maximize a window with the keyboard and have it fill up the screen behind another application on top of it. OK to commit?
Havoc: can I commit the updated patch?
Can you attach the updated one? Or is it all mixed in with your other patch?
Created attachment 17372 [details] [review] sorry -- didn't realize I hadn't attached it :-)
Thanks, looks good to me. Let's put it in.
OK committed.