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

Proposed by Charon
Status: Merged
Approved by: Daniel van Vugt
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 Approve
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.
Revision history for this message
Michael Terry (mterry) wrote :

Seems like we should have a test for this case.

Revision history for this message
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.

Revision history for this message
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
=== modified file 'src/screen.cpp'
--- src/screen.cpp 2012-09-18 01:44:23 +0000
+++ src/screen.cpp 2012-12-08 21:26:22 +0000
@@ -4243,28 +4243,11 @@
42434243
4244 if (strategy == CoreOptions::OverlappingOutputsSmartMode)4244 if (strategy == CoreOptions::OverlappingOutputsSmartMode)
4245 {4245 {
4246 int centerX, centerY;4246 /* We're only going to use geomRect for overlapping area calculations,
42474247 so the window rectangle is enough. We don't need to consider
4248 /* for smart mode, calculate the overlap of the whole rectangle4248 anything more like the border because it will never be significant
4249 with the output device rectangle */4249 to the result */
4250 geomRect.setWidth (gm.width () + 2 * gm.border ());4250 geomRect = gm;
4251 geomRect.setHeight (gm.height () + 2 * gm.border ());
4252
4253 x = gm.x () % screen->width ();
4254 centerX = (x + (geomRect.width () / 2));
4255 if (centerX < 0)
4256 x += screen->width ();
4257 else if (centerX > screen->width ())
4258 x -= screen->width ();
4259 geomRect.setX (x);
4260
4261 y = gm.y () % screen->height ();
4262 centerY = (y + (geomRect.height () / 2));
4263 if (centerY < 0)
4264 y += screen->height ();
4265 else if (centerY > screen->height ())
4266 y -= screen->height ();
4267 geomRect.setY (y);
4268 }4251 }
4269 else4252 else
4270 {4253 {
42714254
=== modified file 'src/window.cpp'
--- src/window.cpp 2012-09-18 15:51:59 +0000
+++ src/window.cpp 2012-12-08 21:26:22 +0000
@@ -3511,71 +3511,20 @@
3511 * but at least the user will be able to see all of the window */3511 * but at least the user will be able to see all of the window */
3512 output = &screen->outputDevs ().at (screen->outputDeviceForGeometry (old));3512 output = &screen->outputDevs ().at (screen->outputDeviceForGeometry (old));
35133513
3514 if (state & CompWindowStateFullscreenMask ||3514 /*
3515 state & CompWindowStateMaximizedHorzMask)3515 * output is now the correct output for the given geometry.
3516 {3516 * There used to be a lot more logic here to handle the rare special
3517 int width = (mask & CWWidth) ? xwc->width : old.width ();3517 * case of maximizing a window whose hints say it is too large to fit
3518 int height = (mask & CWHeight) ? xwc->height : old.height ();3518 * the output and choose a different one. However that logic was a bad
35193519 * idea because:
3520 window->constrainNewWindowSize (width, height, &width, &height);3520 * (1) It's confusing to the user to auto-magically move a window
35213521 * between monitors when they didn't ask for it. So don't.
3522 if (width > output->width ())3522 * (2) In the worst case where the window can't go small enough to fit
3523 {3523 * the output, they can simply move it with Alt+drag, Alt+F7 or
3524 int distance = std::numeric_limits <int>::max ();3524 * expo.
3525 CompOutput *selected = output;3525 * Not moving the window at all is much less annoying than moving it when
3526 /* That's no good ... try and find the closest output device to this one3526 * the user never asked to.
3527 * which has a large enough size */3527 */
3528 foreach (CompOutput &o, screen->outputDevs ())
3529 {
3530 if (o.workArea ().width () > width)
3531 {
3532 int tDistance = sqrt (pow (abs (o.x () - output->x ()), 2) +
3533 pow (abs (o.y () - output->y ()), 2));
3534
3535 if (tDistance < distance)
3536 {
3537 selected = &o;
3538 tDistance = distance;
3539 }
3540 }
3541 }
3542
3543 output = selected;
3544 }
3545 }
3546
3547 if (state & CompWindowStateFullscreenMask ||
3548 state & CompWindowStateMaximizedVertMask)
3549 {
3550 int width = (mask & CWWidth) ? xwc->width : old.width ();
3551 int height = (mask & CWHeight) ? xwc->height : old.height ();
3552
3553 window->constrainNewWindowSize (width, height, &width, &height);
3554
3555 if (height > output->height ())
3556 {
3557 int distance = std::numeric_limits <int>::max ();
3558 CompOutput *selected = output;
3559 /* That's no good ... try and find the closest output device to this one
3560 * which has a large enough size */
3561 foreach (CompOutput &o, screen->outputDevs ())
3562 {
3563 if (o.workArea ().height () > height)
3564 {
3565 int tDistance = sqrt (pow (abs (o.x () - output->x ()), 2) +
3566 pow (abs (o.y () - output->y ()), 2));
3567
3568 if (tDistance < distance)
3569 {
3570 selected = &o;
3571 tDistance = distance;
3572 }
3573 }
3574 }
3575
3576 output = selected;
3577 }
3578 }
35793528
3580 workArea = output->workArea ();3529 workArea = output->workArea ();
35813530

Subscribers

People subscribed via source and target branches