Code review comment for lp:~mc-return/compiz/compiz.merge-fix1082001-gridded-windows-jump-workspaces

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