Bug 157196

JJ: kdebase/workspace/kwin/effects/desktopgrid* - there are few calls to calcDesktopLayout() function that'd need to be wrapped in a function that'd check optional settings and return either those or get the real layout.

Created attachment 25113 [details]
Patch for alternate GridLayout

applies to:
desktopgrid.h
desktopgrid.cpp
desktopgrid_config.h
desktopgrid_config.cpp

Here is a Patch for the alternate Grid layout. I didn't honor the aspect ratio
but i limitet the grid to a minimum of 4 fields so the desks won't get to
skewed. I tested it with 1-20 desks and it seems to work fine.

Regards 
Chris

The patch looks ok to me, I would only name the label something like 'Optimal grid layout' with the opposite meaning to the patch, enabled by default (i.e. do not actually follow pager by default, as for normal numbers of desktops the result should be in practice the same, and otherwise it's better to automatically use a good layout).

However the patch cannot go in now because of 4.1 freeze. Please ping me if I forget the patch after 4.1 release.

I think this should act the opposite way, KWin keeps its layout and the pager uses it's own (I.e. don't apply this patch). If this is added to one effect then it also needs to be added to all others/the effect API as well to keep things consistent.

Adding it to the pager instead means it's possible to have multiple pagers with different amounts of rows (There's also a Plasma wish for that somewhere). If that is the case a row count setting should be added to the desktop system settings panel as the pager will no longer be able to modify it on its own.

I'm not sure if i got your concerns right, but i can't see a problem with the patch. The new option doesn't affect the pager in any way. Without the patch the DesktopGrid effect gets its layout from the pager and not the other way round.
With the patch, the effect calculates now the layout on its own which is the way it should be as i think because the pager hasn't a fixed aspect ratio but the screen obviously has. Therefore it's no use if you have 5 desktops in a row in the effect (because they get totally skewed) which is totally fine for the pager (because the pager just expands horizontally.

So the patch doesn't affect the pager in any way and you can still have multiple pagers with different amounts of rows.

But maybe i just misunderstood you. 

The grid is a visualization of the desktop layout used by KWin, this grid is stored in KWin itself and doesn't need the pager to function. The pager is a separate application that communicates to KWin over DBus to receive and change the layout, as all pagers use the same data they are all linked and cannot have different layouts to each other (If you actually try to do this you can set them up so they look different the first time but the moment you restart Plasma they both get the same layout).

Lets think of this use case:

A user has a pager with a 2x3 layout and we have applied this patch which gives the desktop grid effect a 3x2 layout. The user is currently on the second desktop and executes the "move window to the desktop below". Where does the desktop go? According to the desktop grid effect the desktop below is desktop 5, but the pager says it's desktop 4.

The pager is directly linked to KWin's internal layout so the real desktop is 4 and the desktop grid is using it's own private layout that doesn't match the rest of the system. So what happens when the user doesn't have a pager at all and only uses the desktop grid as a desktop switcher? The user will think that desktop 5 really is below and has absolutely no way of knowing why it was moved to desktop 4 instead.

If the change is applied to the pager instead of using this patch the user WILL know why the pager isn't matching the desktop grid (As the effect will be using the correct layout while the pager is using a private one) as the pager uses the KWin layout by default and the user is required to change the layout explicitly to change the pager layout.

Ok now i understand, you have some valid points but i have some concerns.

Maybe we should just not make the new layout the default option so the user has to change it manually and therefore has the information that the Pager/DesktopGrid layouts aren't consistend anymore also.

For me as a user i wouldn't like it if i had to switch to the desktopgrid to find out if i have to move it to the screen below or the screen right of the current. I always orientated on the pager because it is alway visible(if it's in the panel).

So although i understand your concenrns and agree that it isn't a really clean solution i don't think your solution is better in usability.

Ah, I see the problem for when you use the pager to get the directions. Looks like there is no easy way to accommodate both usage situations.

What about this: As I'm already in the process of revamping the desktop grid what if I add the patch with a little modification--instead of determining the grid layout automatically have the user manually input the amount of rows to display? It's better than doing it automatically as it accommodates more users.

(I should actually mention that I somehow completely missed that your patch had a configuration setting to disable the feature, I thought it was an always-on thing ^_^; )

That's fine for me, but you could also leave the option of the automatic calculation, because for me this works perfectly. This way everybody should be happy. But if you think the automatic calculations could confuse users you can also remove it. Do as you like, it's no big deal for me personally anyway. Thanks for your effort :).

Actually, the description of who decides about the desktop layout is not correct, at least as far as the WM specification is concerned. According to it (http://standards.freedesktop.org/wm-spec/wm-spec-1.3.html#id2506507), it is the pager who sets the global layout and KWin just follows it. The case of two different pagers doesn't seem to be considered.

I still think that for now the best option is applying the patch and having the option enabled by default.

SVN commit 845893 by lmurray:

Improved the desktop grid effect.
FEATURE: 163104, 167265, 168557
CCBUG: 156247, 157196, 158787


 M  +0 -1      COMPOSITE_TODO
 M  +3 -0      effects/CMakeLists.txt
 M  +443 -441  effects/desktopgrid.cpp
 M  +3 -3      effects/desktopgrid.desktop
 M  +33 -22    effects/desktopgrid.h
 M  +122 -66   effects/desktopgrid_config.cpp
 M  +15 -6     effects/desktopgrid_config.h
 A             effects/desktopgrid_config.ui
 A             effects/slide.cpp   [License: GPL (v2+)]
 A             effects/slide.desktop
 A             effects/slide.h   [License: GPL (v2+)]


WebSVN link: http://websvn.kde.org/?view=rev&revision=845893

SVN commit 857049 by lmurray:

Added automatic layout mode to the desktop grid effect. Patch based off one by Christian Mollekopf.
BUG: 157196

 M  +22 -5     desktopgrid.cpp  
 M  +5 -1      desktopgrid.h  
 M  +9 -9      desktopgrid_config.cpp  
 M  +1 -0      desktopgrid_config.h  
 M  +26 -3     desktopgrid_config.ui  


WebSVN link: http://websvn.kde.org/?view=rev&revision=857049

*** Bug 170589 has been marked as a duplicate of this bug. ***

*** Bug 177491 has been marked as a duplicate of this bug. ***