Bug 157196 - JJ: Automatic layout for DesktopGrid
Summary: JJ: Automatic layout for DesktopGrid
Alias: None
Product: kwin
Classification: Plasma
Component: compositing (show other bugs)
Version: unspecified
Platform: unspecified Linux
: NOR wishlist
Target Milestone: ---
Assignee: KWin default assignee
: 170589 177491 (view as bug list)
Depends on:
Reported: 2008-02-05 10:33 UTC by S. Burmeister
Modified: 2008-12-11 14:54 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:

Patch for alternate GridLayout (7.21 KB, patch)
2008-06-04 12:36 UTC, Christian Mollekopf

Note You need to log in before you can comment on or make changes to this bug.
Description S. Burmeister 2008-02-05 10:33:38 UTC
Version:            (using KDE 4.0.0)
OS:                Linux

If one enables the dekstop raster effect (CTRL+F8) the desktops are rastered according to the pager-settings, i.e. in my case, one row, five desktops. This makes the effect unuseable.

As a solution the effect should ignore the pager-settings by default and have a setting to enable that feature.

Or one could simply assume that no user wants desktops to be displayed 5 times as high as wide and check the aspect ratio before drawing the desktops, i.e maintain the aspect ratio at least roughly and arrange the desktops accordingly.
Comment 1 Lubos Lunak 2008-02-05 19:02:33 UTC
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.
Comment 2 Christian Mollekopf 2008-06-04 12:36:29 UTC
Created attachment 25113 [details]
Patch for alternate GridLayout

applies to:

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.

Comment 3 Lubos Lunak 2008-06-10 16:06:16 UTC
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.
Comment 4 lucas 2008-08-02 11:04:46 UTC
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.
Comment 5 Christian Mollekopf 2008-08-02 12:55:36 UTC
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. 
Comment 6 lucas 2008-08-02 13:15:12 UTC
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.
Comment 7 Christian Mollekopf 2008-08-02 13:45:21 UTC
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.
Comment 8 lucas 2008-08-02 14:58:57 UTC
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 ^_^; )
Comment 9 Christian Mollekopf 2008-08-02 15:30:05 UTC
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 :).
Comment 10 Lubos Lunak 2008-08-12 15:40:59 UTC
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.
Comment 11 lucas 2008-08-12 17:28:53 UTC
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
Comment 12 lucas 2008-09-04 16:40:06 UTC
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
Comment 13 lucas 2008-09-07 12:13:49 UTC
*** Bug 170589 has been marked as a duplicate of this bug. ***
Comment 14 lucas 2008-12-11 14:54:34 UTC
*** Bug 177491 has been marked as a duplicate of this bug. ***