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

[MC Return]

Grid:
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.

Those are actually semi-maximized horizontally, so we will treat
them as such and let core handle the restoring, just like we
already do for semi-maximized vertically 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 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.

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:
Tell grid when viewport change is in progress.
(Thanks and credits for this go to Sami Jaktholm)

Move:
Attention: There is no possibility to drag off horizontally resized
windows yet, the fix for this problem is proposed separately.

(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.

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

3653. By MC Return on 2013-04-06

Grid:
Forbid cycling through different sizes for corner and center-gridded
windows also, now fully fixing bug #878820 and following the design
specification for now

TODO: Make the behaviour configurable for the user as many prefer
cycling through different sizes

Added comment
Minor whitespace fixes
Declare CompOutput out and assign a value to it in one line

[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...

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

3654. By MC Return on 2013-04-06

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

3655. By MC Return on 2013-04-06

plugins/grid/grid.xml.in:

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

grid:

Allow cycling for corner/center gridded windows if specified by the
user

[MC Return (mc-return)]

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

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

3656. By MC Return on 2013-04-06

Allow cycling through different sizes for top/bottom and left/right
gridded windows as well, if the user specified it in CCSM explicitely
(default is off)

[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 :)

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

3657. By MC Return on 2013-04-07

Reverted r3652 as this change to the move plugin is not tightly related
to this branch and thus will be proposed seperately to reduce diff size
here

[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

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

3658. By MC Return on 2013-04-07

Grid:
We want to use the same functionality like the unmaximize_or_minimize_window
shortcut normally provides, so if our window has not been touched by grid, we
will do the same with that window that core would normally do
(see src/actions.cpp, unmaximizeOrMinimizeWin and
 launchpad bug LP: #1116538 for details)

Note: If we use the same shortcut for unmaximize_or_minimize_window_key (core)
and put_restore_key (grid) core will take over again, if grid gets disabled
If grid is enabled the unmaximize_or_minimize_window_key will be overridden,
because core always loads first and grid later

Also note that this commit did not change any shortcut yet, but just provides
the functionality for grid - Please use your grid restore keybinding like you
would use the unmaximize_or_minimize_window_key normally to test

3659. By MC Return on 2013-04-09

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

3660. By MC Return on 2013-04-09

Merged Grid from latest lp:compiz

Simplified bool GridScreen::restoreWindow (...):
If Grid has not touched the window we return false and let core do the
dirty work (Thanks go to Sam Spilsbury for suggesting this)

Improved the CCSM grid restore tooltip

3661. By MC Return on 2013-04-09

Merged latest lp:compiz

Unmerged revisions

3661. By MC Return on 2013-04-09

Merged latest lp:compiz

3660. By MC Return on 2013-04-09

Merged Grid from latest lp:compiz

Simplified bool GridScreen::restoreWindow (...):
If Grid has not touched the window we return false and let core do the
dirty work (Thanks go to Sam Spilsbury for suggesting this)

Improved the CCSM grid restore tooltip

3659. By MC Return on 2013-04-09

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

3658. By MC Return on 2013-04-07

Grid:
We want to use the same functionality like the unmaximize_or_minimize_window
shortcut normally provides, so if our window has not been touched by grid, we
will do the same with that window that core would normally do
(see src/actions.cpp, unmaximizeOrMinimizeWin and
 launchpad bug LP: #1116538 for details)

Note: If we use the same shortcut for unmaximize_or_minimize_window_key (core)
and put_restore_key (grid) core will take over again, if grid gets disabled
If grid is enabled the unmaximize_or_minimize_window_key will be overridden,
because core always loads first and grid later

Also note that this commit did not change any shortcut yet, but just provides
the functionality for grid - Please use your grid restore keybinding like you
would use the unmaximize_or_minimize_window_key normally to test

=== 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-07 11:09:23 +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-07 11:09:23 +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-07 11:09:23 +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))
@@ -169,17 +168,16 @@
 	    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)
 	{
@@ -198,7 +196,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 +219,46 @@
 			  (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 (where & ~(GridMaximize |
+			  GridLeft | GridRight | GridTop | GridBottom))
+		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 ());
 
+	/* 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 (desiredRect.y () == currentRect.y () &&
 	    desiredRect.height () == currentRect.height () &&
-	    where & ~(GridMaximize | GridLeft | GridRight) && gw->lastTarget & where)
+	    (where & ~(GridMaximize) ||
+	     (where & ~(GridLeft | GridRight | GridTop | GridBottom) &&
+	      !optionGetCycleSizes ())) &&
+	    gw->lastTarget & where)
 	{
 	    int slotWidth25  = workarea.width () / 4;
 	    int slotWidth33  = (workarea.width () / 3) + cw->border ().left;
@@ -248,8 +274,8 @@
 		    gw->resizeCount = 3;
 
 		/* tricky, have to allow for window constraints when
-		 * computing what the 33% and 66% offsets would be
-		 */
+		     * computing what the 33% and 66% offsets would be
+		     */
 		switch (gw->resizeCount)
 		{
 		    case 1:
@@ -285,7 +311,6 @@
 	    }
 	    else /* keys (2, 5, 8) */
 	    {
-
 		if ((currentRect.width () == desiredRect.width () &&
 		     currentRect.x () == desiredRect.x ()) ||
 		    (gw->resizeCount < 1) || (gw->resizeCount > 5))
@@ -313,7 +338,8 @@
 			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;
@@ -352,37 +378,62 @@
 
 	    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 (where & GridLeft || where & GridRight ||
+		    where & GridTop || where & GridBottom)
+		{
+		    /* 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);
+
+		    if (where & GridLeft || where & GridRight)
+		    {
+			gw->isGridVertMaximized = true;
+			gw->isGridHorzMaximized = false;
+			gw->isGridResized = false;
+
+			/* Semi-maximize the window vertically */
+			cw->maximize (CompWindowStateMaximizedVertMask);
+		    }
+		    else /* GridTop || GridBottom */
+		    {
+			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) -
@@ -694,7 +745,6 @@
 void
 GridScreen::handleEvent (XEvent *event)
 {
-    CompOutput out;
     CompWindow *w;
 
     screen->handleEvent (event);
@@ -706,6 +756,7 @@
 
     currentWorkarea = screen->getWorkareaForOutput
 		      (screen->outputDeviceForPoint (pointerX, pointerY));
+
     if (lastWorkarea != currentWorkarea)
     {
 	lastWorkarea = currentWorkarea;
@@ -719,7 +770,7 @@
 	    cScreen->damageRegion (desiredSlot);
     }
 
-    out = screen->outputDevs ().at (
+    CompOutput out = screen->outputDevs ().at (
 	      screen->outputDeviceForPoint (CompPoint (pointerX, pointerY)));
 
     /* Detect corners first */
@@ -843,8 +894,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 +919,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 +963,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 +997,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 +1036,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 +1046,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 +1086,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 +1110,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 +1128,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 +1291,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-07 11:09:23 +0000
@@ -182,7 +182,8 @@
 	GridScreen *gScreen;
 
 	bool isGridResized;
-	bool isGridSemiMaximized;
+	bool isGridHorzMaximized;
+	bool isGridVertMaximized;
 	unsigned int grabMask;
 	int pointerBufDx;
 	int pointerBufDy;