Bug 222921

Created attachment 39930 [details]
adds neighborhood semantics to kephal and uses this in kwin

1. adds above/left/... semantics to screens in kephal
2. replace buggy code in kwin

probably the neighborhood semantics is too strong. but i think user don't leave any gaps between their monitors. i've tested it with horizontal and vertical stacked and an L-shaped  setup. 

i'm not sure whether u can commit to kephal, but from a software engineer's point of view it's the right place

Could you please open a review request against Kephal on svn.reviewboard.kde.org or git.reviewboard.kde.org (after 29th of January). It's quite a pity that nobody responded to the patch and it was kind of lost in this report :-(

Concerning KWin: the quick tile code should only respect the current screen and not other screens.

Current version of KWin does no longer use Kephal, which means the patch does no longer apply.

Changing to feature request as it needs extension of functionality.

quick note: the attached patch is "wrong" anyway since it assumes the screens would be strictly adjacent.

I *may* have fixed this in a local patch - if you're still interested, please post the output of the problematic "xrandr -q" (rather than a nice but imprecise ascii art ;-)

Git commit d46ca4837da88f6baee16cd38b6be8f7802fac6f by Thomas Lübking.
Committed on 23/12/2013 at 01:28.
Pushed by luebking into branch 'KDE/4.11'.

fix shortcut driven quicktiling

1. swapping direction would rather toggle tiling
2. the next screen was calculated wrongly (found outmost)
3. the electrictborder geometry was not updated when swapping the mode on screen changes
Related: bug 329136
FIXED-IN: 4.11.6
REVIEW: 114648

M  +46   -44   kwin/geometry.cpp

http://commits.kde.org/kde-workspace/d46ca4837da88f6baee16cd38b6be8f7802fac6f

The last commit will show up in kde-workspace 4.11.6 (released in about two weeks) - it would be very kind if you could check whether that fixes your issue as well.

Thomas, thanks for your effort. Am currently not using KDE and therefore I have no development environment either. I'll try to get KDE up and running when the updated packages hit in Fedora.

I know this is not a review board, but i have one annotation and one question:
Line 3219 (and same for line 3221):
if (x >= screens[curScreen].center().x() || (curScreen != nextScreen && x <= screens[nextScreen].center().x()))
If the screens are not the same, then x-coordinates won't be the same, so the second condition can be shortened to:
if (x >= screens[curScreen].center().x() || ( x < screens[nextScreen].center().x()))

Line 3214:
Assuming you have a (highly unlikely) setup of four screens, where one screen has e.g. 4K resolution and three smaller monitors (A,B,C) are stacked to one side, like: 
|A||   |
|B||4k|
|C||   |
So the 4k screen's y-resolution so big, that it will span across A,B and C. Will the tilled window reliably end up on monitor B or does it depend on the placement in the screens vector? The same issue may apply to only two smaller screens next to 4k, but then the placement is non-trivial anyway.

Otherwise the patch looks promising. Keep up the awesome work!

(In reply to comment #7)
> If the screens are not the same, then x-coordinates won't be the same, so

|--------------|
| Screen 1 |
|--------------|
|--------------|
| Screen 2 |
|--------------|

You were saying? ;-)
(Actually the entire center could be the same if the screens are clones)

> Line 3214:
> Assuming you have a (highly unlikely) setup of four screens, where one
> screen has e.g. 4K resolution and three smaller monitors (A,B,C) are stacked
> to one side, like: 

You mean moving the tile from the 4k screen to the left?
All three screens on the left are "valid" candidates - the first one tested (likely "A") will be chosen - the code just checks that the new screen is not really above or really below the old screen.

If you want to "post-review", see https://git.reviewboard.kde.org/r/114648/

> You were saying? ;-)
> (Actually the entire center could be the same if the screens are clones)
But this case is already handled before in line 3214, isn't it? If it's not in the same "stripe", the screen is not considered further.

(In reply to comment #9)
> > You were saying? ;-)
> > (Actually the entire center could be the same if the screens are clones)
> But this case is already handled before in line 3214, isn't it? If it's not
> in the same "stripe", the screen is not considered further.

Notice that the check is *not* for "are x coordinates not equal" but to move the window to the closest left or right screen (depending on the tiling mode), thus has to ensure it moves
a) into the right direction and
b) not further than a position we already may have found.

Hi, I've tried 4.12 and it was working as expected. Thanks!