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 671677 - unmaximizes to nearly maximized size
unmaximizes to nearly maximized size
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-03-08 19:16 UTC by William Jon McCann
Modified: 2012-03-22 17:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Don't unmaximize to nearly maximized size (2.51 KB, patch)
2012-03-12 16:07 UTC, drago01
none Details | Review
Don't unmaximize to nearly maximized size (2.65 KB, patch)
2012-03-12 16:17 UTC, drago01
reviewed Details | Review
Don't unmaximize to nearly maximized size (4.37 KB, patch)
2012-03-12 19:29 UTC, drago01
reviewed Details | Review
Don't unmaximize to nearly maximized size (3.24 KB, patch)
2012-03-15 16:56 UTC, drago01
none Details | Review
window: Automaximize large windows on map (1.98 KB, patch)
2012-03-15 16:57 UTC, drago01
none Details | Review
window: Automaximize large windows on map (1.98 KB, patch)
2012-03-15 16:57 UTC, drago01
none Details | Review
Don't unmaximize to nearly maximized size (3.24 KB, patch)
2012-03-15 16:57 UTC, drago01
none Details | Review
Don't unmaximize to nearly maximized size (3.24 KB, patch)
2012-03-15 17:21 UTC, drago01
reviewed Details | Review
window: Automaximize large windows on map (1.97 KB, patch)
2012-03-15 17:22 UTC, drago01
reviewed Details | Review
window: Automaximize large windows on map (1.98 KB, patch)
2012-03-15 17:25 UTC, drago01
none Details | Review
Don't unmaximize to nearly maximized size (3.25 KB, patch)
2012-03-15 19:43 UTC, drago01
committed Details | Review
Automaximize large windows on map (1.94 KB, patch)
2012-03-15 19:44 UTC, drago01
committed Details | Review

Description William Jon McCann 2012-03-08 19:16:01 UTC
It seems that when we unmaximize a window we try to restore it to the previously used size. This is problematic in certain cases where the restored size is very close to the workarea size (previous maximized size).

This can occur when windows move between physical displays or when explicitly sized by the user.

Basically we don't really want to create windows that are almost maximized in size but not actually maximized. This creates work for the user and makes it very difficult to use and resize manually.

Related to this is 3) in bug 654046 or bug 651075.

Concretely, I think that when we unmaximize a window we should use something like the following logic:

Set the newly unmaximized window size to the previously used size or 80% of the size of the current workarea (attempting to retain natural aspect ratio if possible), whichever is smaller.
Comment 1 drago01 2012-03-12 16:07:23 UTC
Created attachment 209503 [details] [review]
Don't unmaximize to nearly maximized size

Basically we don't really want to create windows that are almost maximized in
size but not actually maximized. This creates work for the user and makes it
very difficult to use and resize manually.

So set the newly unmaximized window size to the previously used size or 80% of the
size of the current workarea (attempting to retain natural aspect ratio if
possible), whichever is smaller.
Comment 2 drago01 2012-03-12 16:17:40 UTC
Created attachment 209504 [details] [review]
Don't unmaximize to nearly maximized size

Basically we don't really want to create windows that are almost maximized in
size but not actually maximized. This creates work for the user and makes it
very difficult to use and resize manually.

So set the newly unmaximized window size to the previously used size or 80% of the size of the current workarea (attempting to retain natural aspect ratio if
possible), whichever is smaller.

---

Check the min size.
Comment 3 Milan Bouchet-Valat 2012-03-12 16:46:24 UTC
I must say that's really great improvement!

Do you think it would also be possible to trigger a maximization when applications set their window sizes to something close to the screen size? For example, Evince can get a default window size from a PDF document to something that's almost as large as the screen. Instead of relying on applications to be overly smart, the WM could take the decision for them.
Comment 4 William Jon McCann 2012-03-12 17:57:31 UTC
That's one thing that Nick proposed in item 3) of bug 654046 . It probably does make sense I think.
Comment 5 Florian Müllner 2012-03-12 18:37:43 UTC
Review of attachment 209504 [details] [review]:

The code looks correct, but I wonder if it's in the right place - if a window maps a window almost at screen size, you have to maximize/unmaximize to get it restricted to a "reasonable" size. Maybe adding a constraint (or modifying an existing one) is a better alternative?

::: src/core/window.c
@@ +3800,3 @@
+       */
+      if (unmaximize_horizontally && unmaximize_vertically &&
+          desired_rect->width * desired_rect->height > work_area.width * work_area.height * 0.8)

should probably use a constant here (MAX_UNMAXIMIZED_WINDOW_AREA?)
Comment 6 drago01 2012-03-12 19:29:14 UTC
Created attachment 209528 [details] [review]
Don't unmaximize to nearly maximized size

Basically we don't really want to create windows that are almost maximized in
size but not actually maximized. This creates work for the user and makes it
very difficult to use and resize manually.

So set the newly unmaximized window size to the previously used size or 80% of the size of the current workarea (attempting to retain natural aspect ratio if
possible), whichever is smaller.

---

Implementation using a constraint.
Comment 7 Florian Müllner 2012-03-13 18:15:41 UTC
Review of attachment 209528 [details] [review]:

Looks good to me - unless we want ·"large" windows automaximized on map instead, feel free to push.

::: src/core/constraints.c
@@ +915,3 @@
+    {
+      float aspect = (float)info->current.height / (float)info->current.width;
+      info->current.width = MAX (work_area.width * 0.8, window->size_hints.min_width);

I still think a #define is better than hardcoding 0.8 twice.
Comment 8 drago01 2012-03-15 16:56:48 UTC
Created attachment 209856 [details] [review]
Don't unmaximize to nearly maximized size

Basically we don't really want to create windows that are almost maximized in
size but not actually maximized. This creates work for the user and makes it
very difficult to use and resize manually.

So set the newly unmaximized window size to the previously used size or 80% of the
size of the current workarea (attempting to retain natural aspect ratio if
possible), whichever is smaller.

---

Use a constant instead of a hardcoded value.
Comment 9 drago01 2012-03-15 16:57:01 UTC
Created attachment 209857 [details] [review]
window: Automaximize large windows on map

Automaximize windows that cover >= 95% of the work area on map.
Comment 10 drago01 2012-03-15 16:57:09 UTC
Created attachment 209858 [details] [review]
window: Automaximize large windows on map

Automaximize windows that cover >= 95% of the work area on map.
Comment 11 drago01 2012-03-15 16:57:54 UTC
Created attachment 209859 [details] [review]
Don't unmaximize to nearly maximized size

Basically we don't really want to create windows that are almost maximized in
size but not actually maximized. This creates work for the user and makes it
very difficult to use and resize manually.

So set the newly unmaximized window size to the previously used size or 80% of the
size of the current workarea (attempting to retain natural aspect ratio if
possible), whichever is smaller.

--- 

Reattach ...
Comment 12 drago01 2012-03-15 17:21:43 UTC
Created attachment 209864 [details] [review]
Don't unmaximize to nearly maximized size

Basically we don't really want to create windows that are almost maximized in
size but not actually maximized. This creates work for the user and makes it
very difficult to use and resize manually.

So set the newly unmaximized window size to the previously used size or 80% of the
size of the current workarea (attempting to retain natural aspect ratio if
possible), whichever is smaller.
Comment 13 drago01 2012-03-15 17:22:06 UTC
Created attachment 209865 [details] [review]
window: Automaximize large windows on map

Automaximize windows that cover > 80% of the work area on map.
Comment 14 drago01 2012-03-15 17:25:48 UTC
Created attachment 209866 [details] [review]
window: Automaximize large windows on map

window: Automaximize large windows on map
    
Automaximize windows that cover > 80% of the work area on map.
    
https://bugzilla.gnome.org/show_bug.cgi?id=671677

(git-bz strikes...)
Comment 15 drago01 2012-03-15 17:27:24 UTC
Comment on attachment 209866 [details] [review]
window: Automaximize large windows on map

or not ...
Comment 16 Florian Müllner 2012-03-15 18:50:32 UTC
Review of attachment 209864 [details] [review]:

::: src/core/window-private.h
@@ +64,3 @@
+/* Windows that unmaximize to a size bigger than that fraction of the workarea
+ * will be scaled down to that size (while maintaining aspect ratio) */
+#define MAX_UNMAXIMIZED_WINDOW_AREA .8

I don't think the define belongs in the header - it is private to window and not interesting to anything outside.

::: src/core/window.c
@@ +3806,3 @@
+              float aspect = (float)desired_rect->height / (float)desired_rect->width;
+              desired_rect->width = MAX (work_area.width * MAX_UNMAXIMIZED_WINDOW_AREA, window->size_hints.min_width);
+              desired_rect->height = MAX (desired_rect->width * aspect, window->size_hints.min_height);

I think we should try to get as close to MAX_UNMAXIMIZED_WINDOW_AREA as possible - here we are way smaller. It is very obvious for the extreme case of desired_rect having the same size as work_area, in which case we get:

if (w * h > w * h * SOME_FACTOR) {
    new_w = w * SOME_FACTOR;
    new_h = new_w * (h / w) = w * SOME_FACTOR * (h / w) = h * SOME_FACTOR;
}

So we end up with the window covering an area of  w * h * SOME_FACTOR * SOME_FACTOR - TL;DR: I really think we want sqrt(SOME_FACTOR) here :)
Comment 17 Florian Müllner 2012-03-15 18:56:23 UTC
Review of attachment 209865 [details] [review]:

Code looks good, but the commit message is pretty poor - the body is just repeating what the subject says, would be good to quickly mention the motivation behind the change there :)

::: src/core/window-private.h
@@ +67,3 @@
 
+/* Windows larger then this fraction of the work area get automaximized on map */
+#define AUTO_MAXIMIZE_MAP_THRESHOLD .8

Again, not sure this belongs in the header. Also, I don't think it makes sense to use different values here, so maybe we shouldn't use separate defines either?
Comment 18 drago01 2012-03-15 19:43:35 UTC
Created attachment 209879 [details] [review]
Don't unmaximize to nearly maximized size

Basically we don't really want to create windows that are almost maximized in
size but not actually maximized. This creates work for the user and makes it
very difficult to use and resize manually.

So set the newly unmaximized window size to the previously used size or 80% of the
size of the current workarea (attempting to retain natural aspect ratio if
possible), whichever is smaller.

---

*) Move constant
*) Use sqrt
Comment 19 drago01 2012-03-15 19:44:04 UTC
Created attachment 209880 [details] [review]
Automaximize large windows on map

Windows that start up in a size that is almost as big as the workarea create
extra work for the user (resizing or maximizing) so save the user's time by
detecting such windowsn and automaxmize them.

---
*) Share constant
*) Improve commit message
Comment 20 Florian Müllner 2012-03-15 20:52:07 UTC
Review of attachment 209879 [details] [review]:

LGTM
Comment 21 Florian Müllner 2012-03-15 20:54:07 UTC
Review of attachment 209880 [details] [review]:

Last line of the commit message: "windowsn" "automaxmize" - good to go with those fixed.
Comment 22 drago01 2012-03-15 21:01:15 UTC
Attachment 209879 [details] pushed as c39998e - Don't unmaximize to nearly maximized size
Attachment 209880 [details] pushed as f2f5008 - Automaximize large windows on map
Comment 23 Julien Olivier 2012-03-22 17:40:12 UTC
Thanks for that! Now, a suggestion: couldn't this also be done when un-maximizing a half-maximized (vertically) window ? In this case, the vertical size of the window should be forced to about 80% of the last-saved window size if needed. The horizontal size, however, could be set to something like 60% of the horizontal size of the screen, at least, so that it becomes obvious that the window is not half-maximized anymore. This is particularily needed because of bug #315647.