Merge lp:~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces into lp:compiz/0.9.10

[MC Return]

*Grid code:

Prevent center and corner gridded windows from jumping viewports.
(Thanks and credits for this go to Sami Jaktholm)

Prevent top and bottom gridded windows from jumping viewports by
making those semi-maximize horizontally.

As those are actually semi-maximized horizontally, we will treat
them as such and let core handle the restoring, just like we
already do for vertically semi-maximized grid windows (left/right).

Now "Strg+Super+Down" will restore top and bottom gridded windows
correctly as well.

Also multiple gridding to top, bottom, left or right will not
overwrite the stored original size anymore.

Restore windows also when workspace switcher (expo) is active.
(Thanks and credits for this go to Sami Jaktholm)

Allow cycling for all gridded windows if explicitly specified by the
user in CCSM.

Forbid cycling through different sizes for corner and center-gridded
windows also per default, now fully fixing bug #878820 and following
the design specification by Ayatana Design there, making behaviour
consistent.

Introduced 3 new bools:
horzMaximizedGridPosition,
vertMaximizedGridPosition,
anyMaximizedGridPosition

Used these bools inside the if condition checks.
Simplified complicated if condition by removing redundant additional
size check.

Cleanup all around.

*Grid xml:

Added cycle_sizes bool option, which allows the user to choose the
prefered behaviour (fixed versus flexible sizes on multiple presses
on the same grid keyboard shortcut).
Default of this option is off, cycling disabled as specified by design.

Added punctuation where missing and removed it where usually is none.
Fixed typos, improved description and titles.

*Expo code:
Tell grid when viewport change is in progress.
(Thanks and credits for this go to Sami Jaktholm)

(fixes: LP: #878820, LP: #879218, LP: #882754 and LP: #1082001,
 partially fixes: #1116538, #1164332)

[MC Return (mc-return)]

Ping ?

[Sam Spilsbury (smspillaz)]

Hey, I'm pretty busy at the moment. I'll get on to this ASAP.

On Thu, Apr 4, 2013 at 12:11 PM, MC Return <email address hidden> wrote:
> Ping ?
> --
> https://code.launchpad.net/~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces/+merge/156663
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces into lp:compiz.

--
Sam Spilsbury

[MC Return (mc-return)]

> Hey, I'm pretty busy at the moment. I'll get on to this ASAP.
>
Please take your time. I am honestly sorry for stressing.
I am just so excited that this MP already fixes 3 IMHO important
Grid usability bugs and so I was curious about your opinion... ;)

I will continue to work on it in the meantime, because I am
confident now that I am able to fully fix the bugs linked here...

Once all the bugs are fixed, I want to continue with optimization
of the logic and splitting out the various codeparts into separate
functions... - but this should probably be made in a separate MP
as this diff is already large enough, which complicates your review
effort (I am sorry for that)...

I can only ensure you that all changes are tested well and extensively
and manually by myself, which reduces the risk of errors or regressions
slipping through undetected ->
http://bazaar.launchpad.net/~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces/revision/3648
is an example for this ;)

Hope that I have all Grid bugs fixed, once you get to the review ;)

[Sami Jaktholm (sjakthol)]

Some thoughts:
1) Top or bottom gridded windows can't be restored by dragging. The window does move up and down but doesn't snap off without pressing restore button/key (seems to be the default behavior for horizontally maximized windows).
2) Code duplication for GridTop || GridBottom case in initiateCommon. This could go like:
    if Top || Bottom || Left || Right
      *restore original geometry and change sizeHints*
      Resized = false

      if Top || Bottom
         maximize(HorzMask)
         HorzMaximized = true
         VertMaximized = false
      else
         maximize(VertMask)
         HorzMaximized = false
         VertMaximized = true

3) The easiest solution to fix bug 1082001 is to make expo send start/end_viewport_switch event at the start/end of viewport switch. Grid listens for those events and knows not to block the window movement while viewport switch is in progress. Currently only wall and rotate are emitting those events although expo and cube are also triggering viewport switches (so cube might need the change too).

The first point definitely needs fixing, the second one is just an opinion...

review: Needs Fixing

[MC Return (mc-return)]

Hey sampo555 :)
Thanks a lot 4 your review, it is very appreciated.

> Some thoughts:
> 1) Top or bottom gridded windows can't be restored by dragging. The window
> does move up and down but doesn't snap off without pressing restore button/key
> (seems to be the default behavior for horizontally maximized windows).
>

I did not notice that problem, because on multimonitor systems it snaps off, once
your pointer crosses monitors when dragging.
I also have "Rotate Cube"->"Edge Flip DnD" enabled, which makes the top/bottom
gridded window restore as well.

But of course this should be no excuse. I'll try to find the problem and fix it.

> 2) Code duplication for GridTop || GridBottom case in initiateCommon. This
> could go like:
> if Top || Bottom || Left || Right
> *restore original geometry and change sizeHints*
> Resized = false
>
> if Top || Bottom
> maximize(HorzMask)
> HorzMaximized = true
> VertMaximized = false
> else
> maximize(VertMask)
> HorzMaximized = false
> VertMaximized = true
>

Yes. Will fix that also (was already on the TODO list).

> 3) The easiest solution to fix bug 1082001 is to make expo send
> start/end_viewport_switch event at the start/end of viewport switch. Grid
> listens for those events and knows not to block the window movement while
> viewport switch is in progress. Currently only wall and rotate are emitting
> those events although expo and cube are also triggering viewport switches (so
> cube might need the change too).
>

Very good observation. Actually awesome, sounds like the key to the solution. \o/
I still was unsure about the best way to approach the problem...
I think I'll have to conduct some experiments with that...

> The first point definitely needs fixing, the second one is just an opinion...
>

All of your points are valid and your review is very welcome.
They'll all get fixed ;).

sampo555, thanks for your involvement with Compiz.
I hope to see some new MPs from you soon...

P.S.: I'll change the status of this MP to "WIP", but additional comments are
always welcome...

[MC Return (mc-return)]

@sampo55:
Did some experiments with making expo send the start/end_viewport_switch event at the start/end of viewport switch, but I was not able to get it right :(
The main new problem I got by doing that, was that Expo drag-and-dropping windows failed to work once Expo announces start_viewport_switch...
I am quite clueless @ the moment, so if you have a working solution or did experiments yourself, please propose a MP, so we can test it...

[Sami Jaktholm (sjakthol)]

I don't have any code to show as I just checked what wall did but expo didn't. As DnD fails, I assume you're emitting those events right when expo starts/quits(?). If you emit the events only when expo calls moveViewport, it shouldn't affect DnD windows (emitting those events equals to situation without grid). So something like this could work:
  screen->handleCompizEvent ("expo","start_viewport_switch", CompOption::Vector());
  screen->moveViewport(...)
  screen->handleCompizEvent ("expo","end_viewport_switch", CompOption::Vector());

I don't have time to test it right now, but I can try it later...

[MC Return (mc-return)]

> I don't have any code to show as I just checked what wall did but expo didn't.
> As DnD fails, I assume you're emitting those events right when expo
> starts/quits(?). If you emit the events only when expo calls moveViewport, it
> shouldn't affect DnD windows (emitting those events equals to situation
> without grid). So something like this could work:
> screen->handleCompizEvent ("expo","start_viewport_switch",
> CompOption::Vector());
> screen->moveViewport(...)
> screen->handleCompizEvent ("expo","end_viewport_switch",
> CompOption::Vector());
>
> I don't have time to test it right now, but I can try it later...

Yeah - that might indeed be the problem. I'll try that and will report here...
Thanks 4 your involvement, sampo555 - and sorry for spelling your synonym
incorrectly before ;)

[Sami Jaktholm (sjakthol)]

That seems to work, but there's still a problem as gridded windows (corners) can't be dragged anywhere in expo (also present in the current trunk). That's because grid ignores the fact that window is dragged in expo (for valid reasons) but still stops the window from moving.

Seems a bit tricky to get this one right :)

[MC Return (mc-return)]

> That seems to work, but there's still a problem as gridded windows (corners)
> can't be dragged anywhere in expo (also present in the current trunk). That's
> because grid ignores the fact that window is dragged in expo (for valid
> reasons) but still stops the window from moving.
>
> Seems a bit tricky to get this one right :)

Hehe, tricky describes it perfectly ;)
It would really be nice if you could propose the expo/cube code, as I did just get
to the point, where gridded corner windows do not jump viewports anymore, but I am
not able to drag any window in expo mode currently, which is ofc inacceptable...

Other observations:
In trunk it is currently possible to drag and drop corner-gridded windows in expo,
just dragging itself is not animated and the window disappears in one viewport and
suddenly pops up in the viewport it is dragged to once you release the mousebutton...

But on multimonitor systems it is not possible to drag the same windows from one
screen to the other, just from viewport to viewport...

[MC Return (mc-return)]

One other point:
Seems the cube does not need any changes, as just rotate cube seems to be responsible
for moving windows...
If you unfold the cube (which is a cube option) and change workspace corner-gridded
windows stay in their position, so it seems no fix is needed here...

[Sami Jaktholm (sjakthol)]

It seems to be very tricky to restore the gridded window inside expo. We can't use pointer location to calculate the restored position as it's absolute to the screen, not relative to expo viewport. We also can't restore it to original location as it might jump anywhere.

So there's pretty much two solutions:
1) Restore the window in place. Problem: the restored window might detach from the pointer, if the it's smaller than the gridded window (depending on where user grabs the window). This can't be easily solved as we don't know where the pointer is relative to the window.
2) Don't restore the window. If the window is moved inside expo, just forgot that it's gridded. This is less likely to cause any issues but it's not consistent with rest of grid behavior or nice thing to do to the user (I wouldn't like it).

Personally I like the first one more as it has a chance to get it right at least some times. The second option just feels a little broken. Anyways, an implementation of the first option can be found from lp:~sjakthol/compiz/grid-expo-interaction. Please test it, and if you think it's good, you can merge it to your branch and start debugging those horizontally maximized windows.

(I'm still the same guy - just did some profile reconfiguration)

[MC Return (mc-return)]

> It seems to be very tricky to restore the gridded window inside expo. We can't
> use pointer location to calculate the restored position as it's absolute to
> the screen, not relative to expo viewport. We also can't restore it to
> original location as it might jump anywhere.
>
> So there's pretty much two solutions:
> 1) Restore the window in place. Problem: the restored window might detach from
> the pointer, if the it's smaller than the gridded window (depending on where
> user grabs the window). This can't be easily solved as we don't know where the
> pointer is relative to the window.
> 2) Don't restore the window. If the window is moved inside expo, just forgot
> that it's gridded. This is less likely to cause any issues but it's not
> consistent with rest of grid behavior or nice thing to do to the user (I
> wouldn't like it).
>
> Personally I like the first one more as it has a chance to get it right at
> least some times. The second option just feels a little broken. Anyways, an
> implementation of the first option can be found from lp:~sjakthol/compiz/grid-
> expo-interaction. Please test it, and if you think it's good, you can merge it
> to your branch and start debugging those horizontally maximized windows.
>
> (I'm still the same guy - just did some profile reconfiguration)

Hi, Sami Jaktholm :)

I've tested your branch now and this looks like another big improvement to me.
Everything works exactly like you described. Top job !

I also think that the solution you found is our best bet at the moment and it
will still affect just corner- and center-gridded windows to restore to original
size when dragged inside expo.

This solution is just slightly inconsistent, compared to the behaviour of grid-
semi-maximized and maximized windows, which will keep their size when dragged to
a new viewport - but it is 100 times better than the version we have in trunk now,
which also acts inconsistent and makes gridded windows jump around and follow the
user's moves in expo...

Restoring the window to original size is always the best solution, because gridding
it back requires just one shortcut, while it can be nerving to have to manually resize
to get back to original size.
Btw, there is still a working workaround for users, who want to move a corner- or center-
gridded window from one workspace to another (rotate to cube face with window) - this
one works perfectly for gridded windows as well...

I still have to apply your other improvements here, and still have to find a solution
for snapping off horizontally maximized windows correctly and easily...

[MC Return (mc-return)]

> Some thoughts:
> 1) Top or bottom gridded windows can't be restored by dragging. The window
> does move up and down but doesn't snap off without pressing restore button/key
> (seems to be the default behavior for horizontally maximized windows).

Yes. The problem is actually hidden in the move plugin. The option SnapoffMaximized ()
does not take care of & CompWindowStateMaximizedHorzMask windows.

You can easily check this wrong behaviour by horizontally maximizing a window by
right-mousebutton clicking the maximize button and you'll get the same behaviour
like for top and bottom gridded windows with this branch here...

Actually I should file another bug and fix that in another branch I guess, but I
do not know, maybe I'll simply attach another bug report here...

[MC Return (mc-return)]

I have now filed bug #1165198 about "Horizontally semi-maximized windows do not snap off easily"...

@Sami Jaktholm: Can you confirm ?

[Sami Jaktholm (sjakthol)]

A second branch might be a better solution. Let's keep this as a fix for grid&expo problem only. Once the other fix lands, I'm ok with this (although I'm probably not the best judge as I have very narrow understanding of how this stuff actually works).

review: Approve

[MC Return (mc-return)]

> A second branch might be a better solution. Let's keep this as a fix for
> grid&expo problem only. Once the other fix lands, I'm ok with this (although
> I'm probably not the best judge as I have very narrow understanding of how
> this stuff actually works).

Okay. The fix for the move snapoff problem should be relatively easy. I already
got a working solution (just a quick hack), but I am confident I can fix this
correctly soonish...

If you are interested: the problem is hidden in static void moveHandleMotionEvent (...)

There:
if (ms->optionGetSnapoffMaximized ())
does not check:
if (w->state () & CompWindowStateMaximizedHorzMask)

and thus Compiz has no strategy to snap off a horizontally semi-maximized window...

[MC Return (mc-return)]

> A second branch might be a better solution. Let's keep this as a fix for
> grid&expo problem only. Once the other fix lands, I'm ok with this (although
> I'm probably not the best judge as I have very narrow understanding of how
> this stuff actually works).

I've uploaded the fix for bug #1165198 to this branch now anyway as it is somehow
related and is very separated fromthe rest (as the fix is in the move plugin).

Re-review and re-test needed ;)

[Sami Jaktholm (sjakthol)]

The change to move code might cause a change in behavior in a corner case. If the window is HorzMaximized but not VertMaximized, the code will never enter the next else if clause (where it would remaximize the window) as it returns in the new else if block. However I haven't found a way to trigger this case so it shouldn't be a problem.

I still think you should split these changes up as the move changes have nothing to do with the original problem.

Other than that, it looks good and works fine.

review: Approve

[MC Return (mc-return)]

Hey Sami :)

Thanks for the review and testing.
I have tested the changes extensively as well and could find no regressions.

/offtopic on
But during testing I found a new bug unfortunately... it seems to have nothing to do with this MP and can be reproduced in trunk as well, but a multimonitor system is needed:

1.Semi-maximize horizontally or maximize a window on monitor 2
2.Change the viewport
3.Switch back to your window with a switcher, or via click on its icon in a dock/launcher

Result: The viewport will change correctly, but the window will move to the main monitor and maximize/semi-maximize horizontally there

Can you reproduce this also (without this branch) ?
/offtopic off

Regarding move: Sure, I can make a new MP for that - my only personal problem is that my SSD is 96,8% full and each
MP with a compiled/tested Compiz build takes another 400MB away ;) - so I am quite handicapped with available space @ the moment...

[MC Return (mc-return)]

I've changed the partial fixes in the commit message to numbers, but I might have to remove the linked bug reports, so launchpad won't auto-track them and change them to "Fix Committed" when committing this to trunk, but I'll better adjust the status of those manually after the merge...

[Sami Jaktholm (sjakthol)]

I don't have a second monitor to test that.

[MC Return (mc-return)]

> I don't have a second monitor to test that.
No problem, seems to be completely unrelated anyway... will check that one @ a later stage...

We are now bailing out if we are repeating the same action:
Probably this would have been enough:

if (gw->lastTarget == where &&
    gw->isGridResized)
    return false

but it works now also (with a little bit of redundancy in the condition check) ;)

I'll have to optimize that a bit...

[MC Return (mc-return)]

Okay, behaviour is configurable now (at least for corner and center gridded windows), so we won't regress for noone...

[MC Return (mc-return)]

Made the behaviour now fully configurable for all gridded windows.

[Sami Jaktholm (sjakthol)]

This has way too much going on. Please split these changes up (one fix per MP).

review: Needs Fixing

[MC Return (mc-return)]

> This has way too much going on. Please split these changes up (one fix per
> MP).

I will explain first what happened after r3652, which is the last revision
you approved...
Btw, there are subcommits, which all have their own diff - making it easier
to follow development:

In
http://bazaar.launchpad.net/~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces/revision/3653
I forbid cycling through different sizes for corner and center-gridded
windows also, now fully fixing bug #878820 and following the design
specification for now.

In
http://bazaar.launchpad.net/~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces/revision/3654
I simplified the condition check to bail out if the last target was the
same like the current one to prevent corner and centered windows from
cycling through sizes, so that it looked like this:

if (gw->lastTarget == where &&
    gw->isGridResized)
    return false;

That means we bail out if the last target is the same as current one and
the window is already grid-resized, so we won't resize a corner/center
gridded window again.

In
http://bazaar.launchpad.net/~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces/revision/3655
I added a cycle_sizes bool option, which allows the user to choose the
prefered behaviour (fixed versus flexible sizes on multiple presses
 on the same grid keyboard shortcut).
Default of this option is off, cycling disabled as specified by design.
This is the relevant part in the xml, the rest are just minor punctuation/
wording fixes:
<option name="cycle_sizes" type="bool">
    <_short>Cycle Through Multiple Sizes</_short>
    <_long>Cycle through multiple different sizes by using the same keyboard shortcut multiple times in a row.</_long>
    <default>false</default>
</option>

The only code change in this revision is this:
/* We do not want to allow cycling through sizes,
 * unless the user explicitely specified that in CCSM */
if (gw->lastTarget == where &&
    gw->isGridResized &&
    !optionGetCycleSizes ())
    return false;

That means we allow to cycle center and corner gridded
windows now, if the user specifies explicitly in CCSM.

In the final revision
http://bazaar.launchpad.net/~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces/revision/3656
I had to implement the same choice for the rest of the
grid windows, namely left/right/top/bottom, which will
act like normal grid windows, if the user selects the
"want to cycle through sizes" option in CCSM

Voila, everything works perfectly :)

[MC Return (mc-return)]

I've separated the change to the move plugin now from the rest.
You can find the MP here:
https://code.launchpad.net/~mc-return/compiz/compiz.merge-fix1165198-horizontally-semimaximized-windows-do-not-snap-off-easily/+merge/157538

[Sam Spilsbury (smspillaz)]

Hey MCR1. Excellent work, here's some comments so far.

8 + CompOption::Vector o(0);
9 dndWindow->ungrabNotify ();
10
11 + screen->handleCompizEvent ("expo", "start_viewport_switch", o);

I believe there is a constant somewhere, either as a static member of CompOption or just as a global called "noOptions". I'd suggest using that.

43 - <_long>Warp and resize windows to fit an imaginary grid.</_long>
44 + <_long>Warp and resize windows to fit an imaginary grid</_long>
45 <category>Window Management</category>
46 <deps>
47 <requirement>
48 @@ -83,7 +83,7 @@
49 <_long>Window resize action</_long>
50 <option name="top_left_corner_action" type="int">
51 <_short>Upper Left Corner</_short>
52 - <_long>Action to be performed when window is dropped on the top left corner</_long>
53 + <_long>Action to be performed when window is dropped on the top left corner.</_long>

Try to avoid mixing formatting changes with substantive changes.

319 - * computing what the 33% and 66% offsets would be
320 - */
321 + * computing what the 33% and 66% offsets would be
322 + */

I think that's probably an undesired formatting change.

Statements like this:

377 + if (where & GridLeft || where & GridRight ||
378 + where & GridTop || where & GridBottom)

And this:

308 + (where & ~(GridMaximize) ||
309 + (where & ~(GridLeft | GridRight | GridTop | GridBottom) &&

Can be combined, eg

unsigned int VerticalGrid = GridLeft | GridRight;
unsigned int HorizontalGrid = GridTop | GridBottom;
unsigned int ConstrainedGrid = VerticalGrid | HorizontalGrid | GridMaximize;

if (where & ~ConstrinedGrid)

...

if (where & VerticalGrid)
{
}
else if (where & HorizontalGrid)
{
}

530 + else if (!gw->isGridResized &&
531 + gw->isGridHorzMaximized &&
532 + !gw->isGridVertMaximized)
533 + {
534 + /* Window has been horizontally maximized by grid. We only need
535 + * to restore Y and height - core handles X and width. */
536 + if (gw->sizeHintsFlags)
537 + gw->window->sizeHints ().flags |= gw->sizeHintsFlags;
538 + xwcm |= CWY | CWHeight;
539 + }
540 + else if (!gw->isGridResized &&
541 + !gw->isGridHorzMaximized &&
542 + gw->isGridVertMaximized)
543 {
544 /* Window has been vertically maximized by grid. We only need
545 - * to restore the X and width - core handles Y and height. */
546 + * to restore X and width - core handles Y and height. */
547 if (gw->sizeHintsFlags)
548 gw->window->sizeHints ().flags |= gw->sizeHintsFlags;
549 xwcm |= CWX | CWWidth;
550 }
551 -
552 - else if (gw->isGridResized && !gw->isGridSemiMaximized)
553 - /* Window is just gridded (top, bottom, center, corners). We
554 - * need to handle everything. */
555 + else if (gw->isGridResized &&
556 + !gw->isGridHorzMaximized &&
557 + !gw->isGridVertMaximized)
558 + /* Window is just gridded (center, corners).
559 + * We need to handle everything. */

It might make sense to nest these blocks.

if (!isGridResized)
{
    assert (isGridHorzMaximized != isGridVertMaximized);

    if (isGridHorzMaximized)
    {
        ...
    }
    else if (isGridVertMaximized)
   ...

[MC Return (mc-return)]

> Hey MCR1. Excellent work, here's some comments so far.
>
Hi Sam. First of all thanks for the review. It is very appreciated.

> 8 + CompOption::Vector o(0);
> 9 dndWindow->ungrabNotify ();
> 10
> 11 + screen->handleCompizEvent ("expo", "start_viewport_switch", o);
>
> I believe there is a constant somewhere, either as a static member of
> CompOption or just as a global called "noOptions". I'd suggest using that.
>
I am not sure where to find that, but I'll search...

> 43 - <_long>Warp and resize windows to fit an imaginary
> grid.</_long>
> 44 + <_long>Warp and resize windows to fit an imaginary
> grid</_long>
> 45 <category>Window Management</category>
> 46 <deps>
> 47 <requirement>
> 48 @@ -83,7 +83,7 @@
> 49 <_long>Window resize action</_long>
> 50 <option name="top_left_corner_action"
> type="int">
> 51 <_short>Upper Left Corner</_short>
> 52 - <_long>Action to be performed when window
> is dropped on the top left corner</_long>
> 53 + <_long>Action to be performed when window
> is dropped on the top left corner.</_long>
>
> Try to avoid mixing formatting changes with substantive changes.
>
I am sorry - I just can't help myself. I want to harmonize how plugins and their
tooltips look & feel, so if I am fixing stuff in the xml - I automatically just
have to fix those also...

> 319 - * computing what the 33% and 66% offsets would be
> 320 - */
> 321 + * computing what the 33% and 66% offsets would be
> 322 + */
>
> I think that's probably an undesired formatting change.
>
Not sure, have to check...

> Statements like this:
>
> 377 + if (where & GridLeft || where & GridRight ||
> 378 + where & GridTop || where & GridBottom)
>
> And this:
>
> 308 + (where & ~(GridMaximize) ||
> 309 + (where & ~(GridLeft | GridRight | GridTop | GridBottom)
> &&
>
> Can be combined, eg
>
> unsigned int VerticalGrid = GridLeft | GridRight;
> unsigned int HorizontalGrid = GridTop | GridBottom;
> unsigned int ConstrainedGrid = VerticalGrid | HorizontalGrid | GridMaximize;
>
> if (where & ~ConstrinedGrid)
>
> ...
>
> if (where & VerticalGrid)
> {
> }
> else if (where & HorizontalGrid)
> {
> }
>
Yeah, I did not touch that any further for the sake of the diffs' size and
readability, but if you want me to fix it in this MP, I'll do it...

> 530 + else if (!gw->isGridResized &&
> 531 + gw->isGridHorzMaximized &&
> 532 + !gw->isGridVertMaximized)
> 533 + {
> 534 + /* Window has been horizontally maximized by grid. We only
> need
> 535 + * to restore Y and height - core handles X and width. */
> 536 + if (gw->sizeHintsFlags)
> 537 + gw->window->sizeHints ().flags |= gw->sizeHintsFlags;
> 538 + xwcm |= CWY | CWHeight;
> 539 + }
> 540 + else if...

[MC Return (mc-return)]

> Hey MCR1. Excellent work, here's some comments so far.
>
> 8 + CompOption::Vector o(0);
> 9 dndWindow->ungrabNotify ();
> 10
> 11 + screen->handleCompizEvent ("expo", "start_viewport_switch", o);
>
> I believe there is a constant somewhere, either as a static member of
> CompOption or just as a global called "noOptions". I'd suggest using that.
>

Hmm, I do not know exactly what to change here, as searching through the code
shows it is almost always used like that, if there are no options - this is from
wall.cpp for example:

    CompOption::Vector o(0);
...
...
    screen->handleCompizEvent ("wall", "start_viewport_switch", o);

I am quite clueless here, but the version we use seems correct...

[MC Return (mc-return)]

> > 319 - * computing what the 33% and 66% offsets would be
> > 320 - */
> > 321 + * computing what the 33% and 66% offsets would
> be
> > 322 + */
> >
> > I think that's probably an undesired formatting change.
> >

Fixed (not yet pushed).

[MC Return (mc-return)]

Sam, it would really be cool if we could merge this at current stage as it is a quasi Grid milestone.

Also I got another branch I stacked on this one:
https://code.launchpad.net/~mc-return/compiz/compiz.merge-fix1116538-isgridresized-windows-do-not-react-to-strg-super-down/+merge/157759

and making too many changes here will complicate further merging :(

I can assure you this here fully works, but I am somehow too lazy to make a video proving that all those bugs are fixed... ;)

Sam, I can assure you I will get to all of your points in a later follow-up MP, but please give this one here a green light first ;)

[Sam Spilsbury (smspillaz)]

Sure, before that, can you:

1. Look into merging some of the bitflags (at the very least, doing if
(where & ~(GridLeft | GridRight | GridTop | GridBottom) is much better
than doing where & ~(GridLeft) || where &~(GridRight) etc
2. Look into whether or not an enum can replace those bools, as they
seem to be mutually exclusive.

On Tue, Apr 9, 2013 at 5:59 AM, MC Return <email address hidden> wrote:
> Sam, it would really be cool if we could merge this at current stage as it is a quasi Grid milestone.
>
> Also I got another branch I stacked on this one:
> https://code.launchpad.net/~mc-return/compiz/compiz.merge-fix1116538-isgridresized-windows-do-not-react-to-strg-super-down/+merge/157759
>
> and making too many changes here will complicate further merging :(
>
> I can assure you this here fully works, but I am somehow too lazy to make a video proving that all those bugs are fixed... ;)
>
> Sam, I can assure you I will get to all of your points in a later follow-up MP, but please give this one here a green light first ;)
> --
> https://code.launchpad.net/~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces/+merge/157760
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces into lp:compiz.

--
Sam Spilsbury

[MC Return (mc-return)]

> Sure, before that, can you:
>
> 1. Look into merging some of the bitflags (at the very least, doing if
> (where & ~(GridLeft | GridRight | GridTop | GridBottom) is much better
> than doing where & ~(GridLeft) || where &~(GridRight) etc
> 2. Look into whether or not an enum can replace those bools, as they
> seem to be mutually exclusive.
>
1 & 2: Done. Even simplified one complicated if condition check additionally.
       I used 3 new bools, like you suggested before - it is much nicer already.
       I think an enum is not necessary here, the bools seem to be enough to
       clean that mess up.

review: Needs Resubmitting

[Sam Spilsbury (smspillaz)]

Coolio, you don't need to resubmit this, I can just reack it when I have
time to look over it next ..

[MC Return (mc-return)]

> Coolio, you don't need to resubmit this, I can just reack it when I have
> time to look over it next ..

I was not sure... I think Daniel once asked me to always resumbmit, or he won't notice it - so I am using this workflow now - I would prefer it to not always have to resubmit, but do not care, if it is prefered to be done this way either...

/offtopic on

I want to create a blueprint on launchpad, where I want to throw in all bugs that are still valid and reproducable.
The bugtracker is a bit of a mess, so I think something like a blueprint combining the most important of them would be
nice...
I want to sort them by: Feature-Requests/Wishlist / Multimonitor only / Regressions / Window-management / Other

Maybe some other categories as well - we'll see...

How do you like that idea ?

/offtopic off

[Sam Spilsbury (smspillaz)]

> /offtopic on
>
> I want to create a blueprint on launchpad, where I want to throw in all bugs
> that are still valid and reproducable.
> The bugtracker is a bit of a mess, so I think something like a blueprint
> combining the most important of them would be
> nice...
> I want to sort them by: Feature-Requests/Wishlist / Multimonitor only /
> Regressions / Window-management / Other
>
> Maybe some other categories as well - we'll see...
>
> How do you like that idea ?
>
> /offtopic off

The best thing to do would be to add them to the 0.9.10.0 milestone. Dunno if that exists yet - I will create it if need be.

[Sam Spilsbury (smspillaz)]

I'm fine with this as is. Sami, what are your thoughts?

review: Approve

[MC Return (mc-return)]

>
> The best thing to do would be to add them to the 0.9.10.0 milestone. Dunno if
> that exists yet - I will create it if need be.

The problem here is that I am not a member of the Compiz team
- so I am not able to change bug priority levels and also cannot add bugs to milestones.

BTW, milestone 0.9.10 already exists:
https://launchpad.net/compiz/+milestone/0.9.10.0

[Sam Spilsbury (smspillaz)]

On Tue, Apr 9, 2013 at 6:04 PM, MC Return <email address hidden> wrote:
>>
>> The best thing to do would be to add them to the 0.9.10.0 milestone. Dunno if
>> that exists yet - I will create it if need be.
>
> The problem here is that I am not a member of the Compiz team
> - so I am not able to change bug priority levels and also cannot add bugs to milestones.
>
> BTW, milestone 0.9.10 already exists:
> https://launchpad.net/compiz/+milestone/0.9.10.0

Okay, we'll need to fix that.

>
> --
> https://code.launchpad.net/~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces/+merge/157817
> You are reviewing the proposed merge of lp:~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces into lp:compiz.

--
Sam Spilsbury

[lp:~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces]

3659. By MC Return on 2013-04-09

Grid xml:
Improved tooltip for the "put_restore_key" reflecting it's enhanced
functionality.

[Sami Jaktholm (sjakthol)]

Looks good and works fine.

review: Approve

[MC Return (mc-return)]

> Looks good and works fine.

Thanks :)

Really time to get this thingy merged ;)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'plugins/expo/src/expo.cpp'
--- plugins/expo/src/expo.cpp	2013-03-26 22:02:39 +0000
+++ plugins/expo/src/expo.cpp	2013-04-09 08:47:24 +0000
@@ -247,10 +247,13 @@
 void
 ExpoScreen::finishWindowMovement ()
 {
+    CompOption::Vector o(0);
     dndWindow->ungrabNotify ();
 
+    screen->handleCompizEvent ("expo", "start_viewport_switch", o);
     screen->moveViewport (screen->vp ().x () - selectedVp.x (),
 			  screen->vp ().y () - selectedVp.y (), true);
+    screen->handleCompizEvent ("expo", "end_viewport_switch", o);
 
     /* update saved window attributes in case we moved the
        window to a new viewport */
@@ -486,6 +489,8 @@
 void
 ExpoScreen::donePaint ()
 {
+    CompOption::Vector o(0);
+    screen->handleCompizEvent ("expo", "start_viewport_switch", o);
     switch (vpUpdateMode) {
     case VPUpdateMouseOver:
 	screen->moveViewport (screen->vp ().x () - selectedVp.x (),
@@ -504,6 +509,7 @@
     default:
 	break;
     }
+    screen->handleCompizEvent ("expo", "end_viewport_switch", o);
 
     if ((expoCam > 0.0f && expoCam < 1.0f) || dndState != DnDNone)
 	cScreen->damageScreen ();

=== modified file 'plugins/grid/grid.xml.in'
--- plugins/grid/grid.xml.in	2013-01-16 23:12:46 +0000
+++ plugins/grid/grid.xml.in	2013-04-09 08:47:24 +0000
@@ -2,7 +2,7 @@
 <compiz>
     <plugin name="grid" useBcop="true">
 	<_short>Grid</_short>
-	<_long>Warp and resize windows to fit an imaginary grid.</_long>
+	<_long>Warp and resize windows to fit an imaginary grid</_long>
 	<category>Window Management</category>
 	<deps>
 	    <requirement>
@@ -83,7 +83,7 @@
 			<_long>Window resize action</_long>
 			<option name="top_left_corner_action" type="int">
 			    <_short>Upper Left Corner</_short>
-			    <_long>Action to be performed when window is dropped on the top left corner</_long>
+			    <_long>Action to be performed when window is dropped on the top left corner.</_long>
 			    <default>4</default>
 			    <min>0</min>
 			    <max>10</max>
@@ -134,7 +134,7 @@
 			</option>
 			<option name="top_edge_action" type="int">
 			    <_short>Top Edge</_short>
-			    <_long>Action to be performed when window is dropped on the top edge</_long>
+			    <_long>Action to be performed when window is dropped on the top edge.</_long>
 			    <default>10</default>
 			    <min>0</min>
 			    <max>10</max>
@@ -185,7 +185,7 @@
 			</option>
 			<option name="top_right_corner_action" type="int">
 			    <_short>Upper Right Corner</_short>
-			    <_long>Action to be performed when window is dropped on the top right corner</_long>
+			    <_long>Action to be performed when window is dropped on the top right corner.</_long>
 			    <default>6</default>
 			    <min>0</min>
 			    <max>10</max>
@@ -236,7 +236,7 @@
 			</option>
 			<option name="left_edge_action" type="int">
 			    <_short>Left Edge</_short>
-			    <_long>Action to be performed when window is dropped on the left edge</_long>
+			    <_long>Action to be performed when window is dropped on the left edge.</_long>
 			    <default>4</default>
 			    <min>0</min>
 			    <max>10</max>
@@ -287,7 +287,7 @@
 			</option>
 			<option name="right_edge_action" type="int">
 			    <_short>Right Edge</_short>
-			    <_long>Action to be performed when window is dropped on the right edge</_long>
+			    <_long>Action to be performed when window is dropped on the right edge.</_long>
 			    <default>6</default>
 			    <min>0</min>
 			    <max>10</max>
@@ -338,7 +338,7 @@
 			</option>
 			<option name="bottom_left_corner_action" type="int">
 			    <_short>Bottom Left Corner</_short>
-			    <_long>Action to be performed when window is dropped on the bottom left corner</_long>
+			    <_long>Action to be performed when window is dropped on the bottom left corner.</_long>
 			    <default>4</default>
 			    <min>0</min>
 			    <max>10</max>
@@ -389,7 +389,7 @@
 			</option>
 			<option name="bottom_edge_action" type="int">
 			    <_short>Bottom Edge</_short>
-			    <_long>Action to be performed when window is dropped on the bottom edge</_long>
+			    <_long>Action to be performed when window is dropped on the bottom edge.</_long>
 			    <default>0</default>
 			    <min>0</min>
 			    <max>10</max>
@@ -440,7 +440,7 @@
 			</option>
 			<option name="bottom_right_corner_action" type="int">
 			    <_short>Bottom Right Corner</_short>
-			    <_long>Action to be performed when window is dropped on the bottom right corner</_long>
+			    <_long>Action to be performed when window is dropped on the bottom right corner.</_long>
 			    <default>6</default>
 			    <min>0</min>
 			    <max>10</max>
@@ -490,42 +490,47 @@
 			    </desc>
 			</option>
 			<option name="snapoff_maximized" type="bool">
-				<_short>Snapoff maximized windows</_short>
-				<_long>Snapoff maximized windows when pulling to edge.</_long>
+				<_short>Snapoff Maximized/Semi-maximized Windows</_short>
+				<_long>Snapoff maximized and vertically maximized windows by dragging them up or down and horizontally maximized ones by dragging them left or right.</_long>
 				<default>false</default>
 			</option>
 			<option name="snapback_windows" type="bool">
-				<_short>Snap windows back to original size</_short>
+				<_short>Snap Windows Back To Original Size</_short>
 				<_long>Snaps windows back to their original size if dragged away from their gridded position.</_long>
 				<default>true</default>
 			</option>
+			<option name="cycle_sizes" type="bool">
+				<_short>Cycle Through Multiple Sizes</_short>
+				<_long>Cycle through multiple different sizes by using the same keyboard shortcut multiple times in a row.</_long>
+				<default>false</default>
+			</option>
 		</subgroup>
 		<subgroup>
 			<_short>Thresholds</_short>
 			<option name="left_edge_threshold" type="int">
 				<_short>Left Edge</_short>
-				<_long>Maximum number of pixels from the left edge a window can be dropped</_long>
+				<_long>Maximum number of pixels from the left edge a window can be dropped.</_long>
 				<default>15</default>
 				<min>0</min>
 				<max>500</max>
 			</option>
 			<option name="right_edge_threshold" type="int">
 				<_short>Right Edge</_short>
-				<_long>Maximum number of pixels from the right edge a window can be dropped</_long>
+				<_long>Maximum number of pixels from the right edge a window can be dropped.</_long>
 				<default>15</default>
 				<min>0</min>
 				<max>500</max>
 			</option>
 			<option name="top_edge_threshold" type="int">
 				<_short>Top Edge</_short>
-				<_long>Maximum number of pixels from the top edge a window can be dropped</_long>
+				<_long>Maximum number of pixels from the top edge a window can be dropped.</_long>
 				<default>20</default>
 				<min>0</min>
 				<max>500</max>
 			</option>
 			<option name="bottom_edge_threshold" type="int">
 				<_short>Bottom Edge</_short>
-				<_long>Maximum number of pixels from the Bottom edge a window can be dropped</_long>
+				<_long>Maximum number of pixels from the bottom edge a window can be dropped.</_long>
 				<default>5</default>
 				<min>0</min>
 				<max>500</max>
@@ -535,24 +540,24 @@
 	    <group>
 		<_short>Appearance</_short>
 		<option name="draw_indicator" type="bool">
-			<_short>Draw Indicator</_short>
-			<_long>Draw Window Resize Indicator</_long>
+			<_short>Preview Indicator</_short>
+			<_long>Draw a window resize indicator preview.</_long>
 			<default>true</default>
 		</option>
 		<option name="draw_stretched_window" type="bool">
-			<_short>Draw Stretched Window</_short>
-			<_long>Draw stretched window animation</_long>
+			<_short>Stretched Live Window Preview</_short>
+			<_long>Draw a stretched live window content animation.</_long>
 			<default>true</default>
 		</option>
 		<option name="animation_duration" type="int">
-			<_short>Animation Duration</_short>
-			<_long>Grid animation duration (in ms)</_long>
+			<_short>Preview Animation Duration</_short>
+			<_long>Preview animation duration (in ms).</_long>
 			<default>350</default>
 			<min>0</min>
 		</option>
 		<option name="outline_color" type="color">
-			<_short>Outline Color</_short>
-			<_long>Color of the resize indicator outline</_long>
+			<_short>Preview Outline Color</_short>
+			<_long>Color and opacity of the resize indicator preview outline.</_long>
 			<default>
 			    <red>0xfbfb</red>
 			    <green>0x8b8b</green>
@@ -561,8 +566,8 @@
 			</default>
 		</option>
 		<option name="fill_color" type="color">
-			<_short>Fill Color</_short>
-			<_long>Fill color of the resize indicator</_long>
+			<_short>Preview Fill Color</_short>
+			<_long>Fill color and opacity of the resize indicator preview.</_long>
 			<default>
 			    <red>0xfbfb</red>
 			    <green>0x8b8b</green>

=== modified file 'plugins/grid/src/grid.cpp'
--- plugins/grid/src/grid.cpp	2013-03-22 11:58:33 +0000
+++ plugins/grid/src/grid.cpp	2013-04-09 08:47:24 +0000
@@ -63,7 +63,6 @@
 			   const CompRect& slot)
 {
     int      cw, ch;
-
     CompRect result = slotToRect (w, slot);
 
     if (w->constrainNewWindowSize (result.width (), result.height (), &cw, &ch))
@@ -137,6 +136,12 @@
 	bool maximizeH = where & (GridBottom | GridTop | GridMaximize);
 	bool maximizeV = where & (GridLeft | GridRight | GridMaximize);
 
+	bool horzMaximizedGridPosition = where & (GridTop | GridBottom);
+	bool vertMaximizedGridPosition = where & (GridLeft | GridRight);
+	bool  anyMaximizedGridPosition = horzMaximizedGridPosition ||
+					 vertMaximizedGridPosition ||
+					 where & GridMaximize;
+
 	if (!(cw->actions () & CompWindowActionResizeMask))
 	    return false;
 
@@ -169,23 +174,23 @@
 	    if (props.numCellsX == 1)
 		centerCheck = true;
 
-	    if (!gw->isGridResized)
+	    /* Do not overwrite the original size if we already have been gridded */
+	    if (!gw->isGridResized && !gw->isGridHorzMaximized && !gw->isGridVertMaximized)
 		/* Store size not including borders when using a keybinding */
 		gw->originalSize = slotToRect(cw, cw->serverBorderRect ());
 	}
 
 	if ((cw->state () & MAXIMIZE_STATE) &&
 	    (resize || optionGetSnapoffMaximized ()))
-	{
 	    /* maximized state interferes with us, clear it */
 	    cw->maximize (0);
-	}
 
 	if ((where & GridMaximize) && resize)
 	{
 	    /* move the window to the correct output */
 	    if (cw == mGrabWindow)
 	    {
+		/* TODO: Remove these magic numbers */
 		xwc.x = workarea.x () + 50;
 		xwc.y = workarea.y () + 50;
 		xwc.width = workarea.width ();
@@ -198,7 +203,9 @@
 	     * gridded one.
 	     */
 	    gw->isGridResized = false;
-	    gw->isGridSemiMaximized = false;
+	    gw->isGridHorzMaximized = false;
+	    gw->isGridVertMaximized = false;
+
 	    for (unsigned int i = 0; i < animations.size (); i++)
 		animations.at (i).fadingOut = true;
 	    return true;
@@ -219,20 +226,43 @@
 			  (workarea.width () / props.numCellsX));
 	desiredSlot.setWidth (workarea.width () / props.numCellsX);
 
-	/* Adjust for constraints and decorations */
-	if (where & ~(GridMaximize | GridLeft | GridRight))
-	    desiredRect = constrainSize (cw, desiredSlot);
-	else
-	    desiredRect = slotToRect (cw, desiredSlot);
+	if (!optionGetCycleSizes ())
+	{
+	    /* Adjust for constraints and decorations */
+	    if (!anyMaximizedGridPosition)
+		desiredRect = constrainSize (cw, desiredSlot);
+	    else
+		desiredRect = slotToRect (cw, desiredSlot);
+	}
+	else /* (optionGetCycleSizes ()) */
+	{
+	    /* Adjust for constraints and decorations */
+	    if (where & ~GridMaximize)
+		desiredRect = constrainSize (cw, desiredSlot);
+	    else
+		desiredRect = slotToRect (cw, desiredSlot);
+	}
 
 	/* Get current rect not including decorations */
 	currentRect.setGeometry (cw->serverX (), cw->serverY (),
 				 cw->serverWidth (),
 				 cw->serverHeight ());
 
-	if (desiredRect.y () == currentRect.y () &&
-	    desiredRect.height () == currentRect.height () &&
-	    where & ~(GridMaximize | GridLeft | GridRight) && gw->lastTarget & where)
+	/* We do not want to allow cycling through sizes,
+	 * unless the user explicitely specified that in CCSM
+	 */
+	if (gw->lastTarget == where &&
+	    gw->isGridResized &&
+	    !optionGetCycleSizes ())
+	    return false;
+
+	/* !(Grid Left/Right/Top/Bottom) are only valid here, if
+	 * cycling through sizes is disabled also
+	 */
+	if ((where & ~(GridMaximize) ||
+	     ((!horzMaximizedGridPosition || !vertMaximizedGridPosition) &&
+	      !optionGetCycleSizes ())) &&
+	    gw->lastTarget & where)
 	{
 	    int slotWidth25  = workarea.width () / 4;
 	    int slotWidth33  = (workarea.width () / 3) + cw->border ().left;
@@ -258,34 +288,38 @@
 					  props.gravityRight * slotWidth33);
 			gw->resizeCount++;
 			break;
+
 		    case 2:
 			gw->resizeCount++;
 			break;
+
 		    case 3:
 			desiredSlot.setWidth (slotWidth33);
 			desiredSlot.setX (workarea.x () +
 					  props.gravityRight * slotWidth66);
 			gw->resizeCount++;
 			break;
+
 		    case 4:
 			desiredSlot.setWidth (slotWidth25);
 			desiredSlot.setX (workarea.x () +
 					  props.gravityRight * slotWidth75);
 			gw->resizeCount++;
 			break;
+
 		    case 5:
 			desiredSlot.setWidth (slotWidth75);
 			desiredSlot.setX (workarea.x () +
 					  props.gravityRight * slotWidth25);
 			gw->resizeCount++;
 			break;
+
 		    default:
 			break;
 		}
 	    }
 	    else /* keys (2, 5, 8) */
 	    {
-
 		if ((currentRect.width () == desiredRect.width () &&
 		     currentRect.x () == desiredRect.x ()) ||
 		    (gw->resizeCount < 1) || (gw->resizeCount > 5))
@@ -299,6 +333,7 @@
 			desiredSlot.setX (workarea.x () + slotWidth17);
 			gw->resizeCount++;
 			break;
+
 		    case 2:
 			desiredSlot.setWidth ((slotWidth25 * 2) +
 					      (slotWidth17 * 2));
@@ -306,20 +341,25 @@
 					  (slotWidth25 - slotWidth17));
 			gw->resizeCount++;
 			break;
+
 		    case 3:
 			desiredSlot.setWidth ((slotWidth25 * 2));
 			desiredSlot.setX (workarea.x () + slotWidth25);
 			gw->resizeCount++;
 			break;
+
 		    case 4:
 			desiredSlot.setWidth (slotWidth33 -
-					      (cw->border ().left + cw->border ().right));
+					      (cw->border ().left +
+					       cw->border ().right));
 			desiredSlot.setX (workarea.x () + slotWidth33);
 			gw->resizeCount++;
 			break;
+
 		    case 5:
 			gw->resizeCount++;
 			break;
+
 		    default:
 			break;
 		}
@@ -352,37 +392,63 @@
 
 	    gw->sizeHintsFlags = 0;
 
-	    /* Special case for left and right, actually vertically maximize
-	     * the window */
-	    if (where & GridLeft || where & GridRight)
+	    if (!optionGetCycleSizes ())
 	    {
-		/* First restore the window to its original size */
-		XWindowChanges rwc;
-
-		rwc.x = gw->originalSize.x ();
-		rwc.y = gw->originalSize.y ();
-		rwc.width = gw->originalSize.width ();
-		rwc.height = gw->originalSize.height ();
-
-		cw->configureXWindow (CWX | CWY | CWWidth | CWHeight, &rwc);
-
-		gw->isGridSemiMaximized = true;
-		gw->isGridResized = false;
-
-		/* Maximize the window */
-		cw->maximize (CompWindowStateMaximizedVertMask);
-
-		/* Be evil */
-		if (cw->sizeHints ().flags & PResizeInc)
-		{
-		    gw->sizeHintsFlags |= PResizeInc;
-		    gw->window->sizeHints ().flags &= ~(PResizeInc);
+		/* Special cases for left/right and top/bottom gridded windows, where we
+		 * actually vertically respective horizontally semi-maximize the window
+		 */
+		if (horzMaximizedGridPosition || vertMaximizedGridPosition)
+		{
+		    /* First restore the window to its original size */
+		    XWindowChanges rwc;
+
+		    rwc.x = gw->originalSize.x ();
+		    rwc.y = gw->originalSize.y ();
+		    rwc.width = gw->originalSize.width ();
+		    rwc.height = gw->originalSize.height ();
+
+		    cw->configureXWindow (CWX | CWY | CWWidth | CWHeight, &rwc);
+
+		    /* GridLeft || GridRight */
+		    if (vertMaximizedGridPosition)
+		    {
+			gw->isGridVertMaximized = true;
+			gw->isGridHorzMaximized = false;
+			gw->isGridResized = false;
+
+			/* Semi-maximize the window vertically */
+			cw->maximize (CompWindowStateMaximizedVertMask);
+		    }
+		    /* GridTop || GridBottom */
+		    else /* (horzMaximizedGridPosition) */
+		    {
+			gw->isGridHorzMaximized = true;
+			gw->isGridVertMaximized = false;
+			gw->isGridResized = false;
+
+			/* Semi-maximize the window horizontally */
+			cw->maximize (CompWindowStateMaximizedHorzMask);
+		    }
+
+		    /* Be evil */
+		    if (cw->sizeHints ().flags & PResizeInc)
+		    {
+			gw->sizeHintsFlags |= PResizeInc;
+			gw->window->sizeHints ().flags &= ~(PResizeInc);
+		    }
+		}
+		else /* GridCorners || GridCenter */
+		{
+		    gw->isGridResized = true;
+		    gw->isGridHorzMaximized = false;
+		    gw->isGridVertMaximized = false;
 		}
 	    }
-	    else
+	    else /* if (optionGetCycleSizes ()) */
 	    {
 		gw->isGridResized = true;
-		gw->isGridSemiMaximized = false;
+		gw->isGridHorzMaximized = false;
+		gw->isGridVertMaximized = false;
 	    }
 
 	    int dw = (lastBorder.left + lastBorder.right) -
@@ -657,31 +723,40 @@
 {
     int ret;
 
-    switch (edge) {
+    switch (edge)
+    {
 	case Left:
 	    ret = (int) optionGetLeftEdgeAction ();
 	    break;
+
 	case Right:
 	    ret = (int) optionGetRightEdgeAction ();
 	    break;
+
 	case Top:
 	    ret = (int) optionGetTopEdgeAction ();
 	    break;
+
 	case Bottom:
 	    ret = (int) optionGetBottomEdgeAction ();
 	    break;
+
 	case TopLeft:
 	    ret = (int) optionGetTopLeftCornerAction ();
 	    break;
+
 	case TopRight:
 	    ret = (int) optionGetTopRightCornerAction ();
 	    break;
+
 	case BottomLeft:
 	    ret = (int) optionGetBottomLeftCornerAction ();
 	    break;
+
 	case BottomRight:
 	    ret = (int) optionGetBottomRightCornerAction ();
 	    break;
+
 	case NoEdge:
 	default:
 	    ret = -1;
@@ -694,7 +769,6 @@
 void
 GridScreen::handleEvent (XEvent *event)
 {
-    CompOutput out;
     CompWindow *w;
 
     screen->handleEvent (event);
@@ -706,6 +780,7 @@
 
     currentWorkarea = screen->getWorkareaForOutput
 		      (screen->outputDeviceForPoint (pointerX, pointerY));
+
     if (lastWorkarea != currentWorkarea)
     {
 	lastWorkarea = currentWorkarea;
@@ -719,7 +794,7 @@
 	    cScreen->damageRegion (desiredSlot);
     }
 
-    out = screen->outputDevs ().at (
+    CompOutput out = screen->outputDevs ().at (
 	      screen->outputDeviceForPoint (CompPoint (pointerX, pointerY)));
 
     /* Detect corners first */
@@ -843,8 +918,8 @@
 
     /* Don't allow non-pagers to change
      * the size of the window, the user
-     * specified this size, thank-you */
-    if (isGridSemiMaximized)
+     * specified this size */
+    if (isGridHorzMaximized || isGridVertMaximized)
 	if (source != ClientTypePager)
 	    xwcm = 0;
 }
@@ -868,7 +943,10 @@
 	pointerBufDx = pointerBufDy = 0;
 	grabMask = mask;
 
-	if (!isGridResized && !isGridSemiMaximized && gScreen->optionGetSnapbackWindows ())
+	if (!isGridResized &&
+	    !isGridHorzMaximized &&
+	    !isGridVertMaximized &&
+	    gScreen->optionGetSnapbackWindows ())
 	    /* Store size not including borders when grabbing with cursor */
 	    originalSize = gScreen->slotToRect(window,
 					       window->serverBorderRect ());
@@ -909,8 +987,18 @@
 {
     window->moveNotify (dx, dy, immediate);
 
-    if (isGridResized && !isGridSemiMaximized && !GridScreen::get (screen)->mSwitchingVp)
+    if (isGridResized &&
+	!isGridHorzMaximized &&
+	!isGridVertMaximized &&
+	!GridScreen::get (screen)->mSwitchingVp)
     {
+	if (window->grabbed () && screen->grabExist ("expo"))
+	{
+	    /* Window is being dragged in expo. Restore the original geometry
+	     * right away to avoid any confusion. */
+	    gScreen->restoreWindow (0, 0, gScreen->o);
+	    return;
+	}
 	if (window->grabbed () && (grabMask & CompWindowGrabMoveMask))
 	{
 	    pointerBufDx += dx;
@@ -933,7 +1021,10 @@
 	!(window->state () & MAXIMIZE_STATE))
     {
 	lastTarget = GridUnknown;
-	if (isGridSemiMaximized && (lastState & MAXIMIZE_STATE) == CompWindowStateMaximizedVertMask)
+	if ((isGridHorzMaximized &&
+	     (lastState & MAXIMIZE_STATE) == CompWindowStateMaximizedHorzMask) ||
+	    (isGridVertMaximized &&
+	     (lastState & MAXIMIZE_STATE) == CompWindowStateMaximizedVertMask))
 	    gScreen->restoreWindow(0, 0, gScreen->o);
     }
     else if (!(lastState & MAXIMIZE_STATE) &&
@@ -969,7 +1060,9 @@
 
     GRID_WINDOW (cw);
 
-    if (!gw->isGridResized && !gw->isGridSemiMaximized)
+    if (!gw->isGridResized &&
+	!gw->isGridHorzMaximized &&
+	!gw->isGridVertMaximized)
     {
 	/* Grid hasn't touched this window or has maximized it. If it's
 	 * maximized, unmaximize it and get out. */
@@ -977,25 +1070,38 @@
 	    cw->maximize(0);
 	return true;
     }
-
-    else if (!gw->isGridResized && gw->isGridSemiMaximized)
+    else if (!gw->isGridResized &&
+	     gw->isGridHorzMaximized &&
+	     !gw->isGridVertMaximized)
+    {
+	/* Window has been horizontally maximized by grid. We only need
+	 * to restore Y and height - core handles X and width. */
+	if (gw->sizeHintsFlags)
+	    gw->window->sizeHints ().flags |= gw->sizeHintsFlags;
+	xwcm |=  CWY | CWHeight;
+    }
+    else if (!gw->isGridResized &&
+	     !gw->isGridHorzMaximized &&
+	     gw->isGridVertMaximized)
     {
 	/* Window has been vertically maximized by grid. We only need
-	 * to restore the X and width - core handles Y and height. */
+	 * to restore X and width - core handles Y and height. */
 	if (gw->sizeHintsFlags)
 	    gw->window->sizeHints ().flags |= gw->sizeHintsFlags;
 	xwcm |= CWX | CWWidth;
     }
-
-    else if (gw->isGridResized && !gw->isGridSemiMaximized)
-	/* Window is just gridded (top, bottom, center, corners). We
-	 * need to handle everything. */
+    else if (gw->isGridResized &&
+	     !gw->isGridHorzMaximized &&
+	     !gw->isGridVertMaximized)
+	/* Window is just gridded (center, corners).
+	 * We need to handle everything. */
 	xwcm |= CWX | CWY | CWWidth | CWHeight;
     else
     {
 	/* This should never happen. But if it does, just bail out
 	 * gracefully. */
-	assert (gw->isGridResized && gw->isGridSemiMaximized);
+	assert (gw->isGridResized &&
+		(gw->isGridHorzMaximized || gw->isGridVertMaximized));
 	return false;
     }
 
@@ -1004,6 +1110,16 @@
 	xwc.x = pointerX - (gw->originalSize.width () / 2);
 	xwc.y = pointerY + (cw->border ().top / 2);
     }
+    else if (cw->grabbed () && screen->grabExist ("expo"))
+    {
+	/* We're restoring a window inside expo by dragging. This is a bit
+	 * tricky. Pointer location is absolute to the screen, not relative
+	 * to expo viewport. So we can't use pointer location to calculate
+	 * the position of the restore window.
+	 *
+	 * The best solution is to resize it in place. */
+	xwcm = CWWidth | CWHeight;
+    }
     else
     {
 	xwc.x = gw->originalSize.x ();
@@ -1018,7 +1134,8 @@
     gw->currentSize = CompRect ();
     gw->pointerBufDx = 0;
     gw->pointerBufDy = 0;
-    gw->isGridSemiMaximized = false;
+    gw->isGridHorzMaximized = false;
+    gw->isGridVertMaximized = false;
     gw->isGridResized = false;
     if (cw->state () & MAXIMIZE_STATE)
 	cw->maximize(0);
@@ -1035,7 +1152,8 @@
     GRID_WINDOW (screen->findWindow
 		 (CompOption::getIntOptionNamed (o, "window")));
     gw->isGridResized = false;
-    gw->isGridSemiMaximized = false;
+    gw->isGridHorzMaximized = false;
+    gw->isGridVertMaximized = false;
     gw->resizeCount = 0;
 }
 
@@ -1197,7 +1315,8 @@
     gWindow (GLWindow::get(window)),
     gScreen (GridScreen::get (screen)),
     isGridResized (false),
-    isGridSemiMaximized (false),
+    isGridHorzMaximized (false),
+    isGridVertMaximized (false),
     grabMask (0),
     pointerBufDx (0),
     pointerBufDy (0),

=== modified file 'plugins/grid/src/grid.h'
--- plugins/grid/src/grid.h	2013-01-21 15:17:50 +0000
+++ plugins/grid/src/grid.h	2013-04-09 08:47:24 +0000
@@ -182,7 +182,8 @@
 	GridScreen *gScreen;
 
 	bool isGridResized;
-	bool isGridSemiMaximized;
+	bool isGridHorzMaximized;
+	bool isGridVertMaximized;
 	unsigned int grabMask;
 	int pointerBufDx;
 	int pointerBufDy;