Merge lp:~charon030/compiz/fix-751605 into lp:compiz/0.9.8

Proposed by Charon on 2012-12-08
Status: Merged
Approved by: Daniel van Vugt on 2012-12-11
Approved revision: 3427
Merged at revision: 3428
Proposed branch: lp:~charon030/compiz/fix-751605
Merge into: lp:compiz/0.9.8
Diff against target: 126 lines (+19/-87)
2 files modified
src/screen.cpp (+5/-22)
src/window.cpp (+14/-65)
To merge this branch: bzr merge lp:~charon030/compiz/fix-751605
Reviewer Review Type Date Requested Status
Daniel van Vugt 2012-12-08 Approve on 2012-12-11
Review via email: mp+138867@code.launchpad.net

Commit message

Prevent windows from maximizing on the wrong monitor (LP: #751605)

  This fixes two causes I have found:
    1. outputDeviceForGeometry was wrapping to the wrong monitor (back to the
       top or left) if a window was mostly off the bottom/right of the screen.
    2. Even when outputDeviceForGeometry returned the correct output,
       PrivateWindow::addWindowSizeChanges was sometimes changing it to an
       incorrect output in the case where the old size of a window exceeds
       the dimensions of the smaller monitor you're trying to maximize it on.
  .

Description of the change

This is the same bugfix as it was integrated into mainline by Daniel van Vugt before in:
https://code.launchpad.net/~vanvugt/compiz/fix-751605

To post a comment you must log in.
Michael Terry (mterry) wrote :

Seems like we should have a test for this case.

Daniel van Vugt (vanvugt) wrote :

The automated tests are in the upstream version (lp:compiz) and should not be backported here because they required some architectural change.

Daniel van Vugt (vanvugt) wrote :

Yep, it's identical to my upstream fix. Just minus the test cases which are not feasible to backport due to architectural changes.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/screen.cpp'
2--- src/screen.cpp 2012-09-18 01:44:23 +0000
3+++ src/screen.cpp 2012-12-08 21:26:22 +0000
4@@ -4243,28 +4243,11 @@
5
6 if (strategy == CoreOptions::OverlappingOutputsSmartMode)
7 {
8- int centerX, centerY;
9-
10- /* for smart mode, calculate the overlap of the whole rectangle
11- with the output device rectangle */
12- geomRect.setWidth (gm.width () + 2 * gm.border ());
13- geomRect.setHeight (gm.height () + 2 * gm.border ());
14-
15- x = gm.x () % screen->width ();
16- centerX = (x + (geomRect.width () / 2));
17- if (centerX < 0)
18- x += screen->width ();
19- else if (centerX > screen->width ())
20- x -= screen->width ();
21- geomRect.setX (x);
22-
23- y = gm.y () % screen->height ();
24- centerY = (y + (geomRect.height () / 2));
25- if (centerY < 0)
26- y += screen->height ();
27- else if (centerY > screen->height ())
28- y -= screen->height ();
29- geomRect.setY (y);
30+ /* We're only going to use geomRect for overlapping area calculations,
31+ so the window rectangle is enough. We don't need to consider
32+ anything more like the border because it will never be significant
33+ to the result */
34+ geomRect = gm;
35 }
36 else
37 {
38
39=== modified file 'src/window.cpp'
40--- src/window.cpp 2012-09-18 15:51:59 +0000
41+++ src/window.cpp 2012-12-08 21:26:22 +0000
42@@ -3511,71 +3511,20 @@
43 * but at least the user will be able to see all of the window */
44 output = &screen->outputDevs ().at (screen->outputDeviceForGeometry (old));
45
46- if (state & CompWindowStateFullscreenMask ||
47- state & CompWindowStateMaximizedHorzMask)
48- {
49- int width = (mask & CWWidth) ? xwc->width : old.width ();
50- int height = (mask & CWHeight) ? xwc->height : old.height ();
51-
52- window->constrainNewWindowSize (width, height, &width, &height);
53-
54- if (width > output->width ())
55- {
56- int distance = std::numeric_limits <int>::max ();
57- CompOutput *selected = output;
58- /* That's no good ... try and find the closest output device to this one
59- * which has a large enough size */
60- foreach (CompOutput &o, screen->outputDevs ())
61- {
62- if (o.workArea ().width () > width)
63- {
64- int tDistance = sqrt (pow (abs (o.x () - output->x ()), 2) +
65- pow (abs (o.y () - output->y ()), 2));
66-
67- if (tDistance < distance)
68- {
69- selected = &o;
70- tDistance = distance;
71- }
72- }
73- }
74-
75- output = selected;
76- }
77- }
78-
79- if (state & CompWindowStateFullscreenMask ||
80- state & CompWindowStateMaximizedVertMask)
81- {
82- int width = (mask & CWWidth) ? xwc->width : old.width ();
83- int height = (mask & CWHeight) ? xwc->height : old.height ();
84-
85- window->constrainNewWindowSize (width, height, &width, &height);
86-
87- if (height > output->height ())
88- {
89- int distance = std::numeric_limits <int>::max ();
90- CompOutput *selected = output;
91- /* That's no good ... try and find the closest output device to this one
92- * which has a large enough size */
93- foreach (CompOutput &o, screen->outputDevs ())
94- {
95- if (o.workArea ().height () > height)
96- {
97- int tDistance = sqrt (pow (abs (o.x () - output->x ()), 2) +
98- pow (abs (o.y () - output->y ()), 2));
99-
100- if (tDistance < distance)
101- {
102- selected = &o;
103- tDistance = distance;
104- }
105- }
106- }
107-
108- output = selected;
109- }
110- }
111+ /*
112+ * output is now the correct output for the given geometry.
113+ * There used to be a lot more logic here to handle the rare special
114+ * case of maximizing a window whose hints say it is too large to fit
115+ * the output and choose a different one. However that logic was a bad
116+ * idea because:
117+ * (1) It's confusing to the user to auto-magically move a window
118+ * between monitors when they didn't ask for it. So don't.
119+ * (2) In the worst case where the window can't go small enough to fit
120+ * the output, they can simply move it with Alt+drag, Alt+F7 or
121+ * expo.
122+ * Not moving the window at all is much less annoying than moving it when
123+ * the user never asked to.
124+ */
125
126 workArea = output->workArea ();
127

Subscribers

People subscribed via source and target branches