[workspace switcher] Inconsistency between workspace layout in graphical switcher and that seen when using keyboard shortcuts

[Jochen Skulj]

You can use the keyboard for switching between the workspaces. E.g. you can configure keys to switch to the workspace left or right to your current workspace.

When you switch between the workspaces using the keyboard, all 4 workspaces are shown in a single row (see attachment SwitchingWorkplaces.png). However, the Unity workplace switcher displays the workspaces in a 2x2 layout (see attachment UnityWorkplaceSwitcher.png).

I think this is a little bit confusing and makes it sometimes difficult to find the right workplace when you use the keyboard control. In case of switching between the workplaces via keyboard the workplaces should also be displayed in a 2x2 layout.

[Jonas Diaz (jonasdiaz)]

Hi Jochen, Nice spotted. I think that they could also be numbered (In both cases, in the 2x2 layout and the horizontal one, or even the fixed one with just the 2x2's) , of course, following the Ubuntu desing and letter format.

[Florian Boucault (fboucault)]

bug #799800 should be fixed as a consequence.

[Justyn Butler (justyn)]

Just tested with latest updates, still seeing this bug.
Unity workspace switcher shows 2x2 array of workspace, switching using the keyboard shows a 1x4 array.

[Justyn Butler (justyn)]

I think the title of this bug is ambiguous and should be changed.

The problem is that with a default Unity 2D setup the workspace layout shown in the graphical unity workspace switcher is 2x2, but when switching with the keyboard it is shown in a 1x4 array. The current title doesn't reflect that.

[Curtis Hovey (sinzui)]

From the duplicate:
@Olivier Tilloy (osomon) wrote on 2011-05-06:
Note that configuring the workspaces layout involves two separate mechanisms:

 - the total number of workspaces, set by metacity and stored in the GConf key /apps/metacity/general/num_workspaces

 - the layout of the desktop, set using the _NET_DESKTOP_LAYOUT atom on the root window (see http://standards.freedesktop.org/wm-spec/wm-spec-latest.html#id2550891)

[Curtis Hovey (sinzui)]

I think unity-2d should set the _NET_DESKTOP_LAYOUT atom on the root window when metacity is running to ensure the layout is as it claims.

[Curtis Hovey (sinzui)]

Attached is a python script that will fix Metacity's layout of the workspaces. This is a practical hack. You can add this to Startup Application if you always run unity-2d with Metacity window manager. I am running it as needed since I use Unity most of the time.

This hack illustrates what unity-2d must do to cooperate with Metacity.

[Jan Kaláb (pitel)]

I just tested gnome-shell and returned to unity2d, I found just 2 workspaces. Thanks for mentioning /apps/metacity/general/num_workspaces in gconf here. It fixed my problem.

[Jouni Roivas (jroivas)]

Attached a little patch against unity-2d which updates _NET_DESKTOP_LAYOUT atom back to the root window as proposed. It seems to fix this issue on my installation.
However this patch won't still fix a problem when changing gconf key /apps/metacity/general/num_workspace to another value. Changing that value does not automatically update layout right away on Unity but needs a restart (for example logout - login).

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "unity-2d-workspaces-update.patch" of this bug report has been identified as being a patch. The ubuntu-reviewers team has been subscribed to the bug report so that they can review the patch. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-sponsors please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

[Jouni Roivas (jroivas)]

Please see revised patch which keeps number of workspaces in sync. It makes assumptions about row and columns on Unity side by the number of workspaces. Now one can change the gconf value at /apps/metacity/general/num_workspace and Unity-2d should update its contents and contents of the switcher accordingly.

[Florian Boucault (fboucault)]

Thanks a lot for that patch Jouni.

Some functional testing shows that it works pretty reliably, thank you.
Let's dig deep in the code side of things.

foreword: below _NET_NUMBER_OF_DESKTOPS and _NET_DESKTOP_LAYOUT refer to the root window's X properties

WorkspacesInfo::updateWorkspaceGeometry() has 3 functions:
- deciding on the number of workspaces (_NET_NUMBER_OF_DESKTOPS) if not done so by an external program (e.g. metacity)
- deciding on the layout of the workspaces (_NET_DESKTOP_LAYOUT) if not done so by an external program (often called pager) (e.g. gnome-panel)
- maintaining WorskpacesInfo's properties 'count', 'rows', 'columns', 'orientation' and 'startingCorner' up to date and in sync with _NET_NUMBER_OF_DESKTOPS and _NET_DESKTOP_LAYOUT

As it is in trunk the code has the following issues:
- if _NET_NUMBER_OF_DESKTOPS is not set and WorkspacesInfo::updateWorkspaceGeometry decides to default to 4, the property _NET_NUMBER_OF_DESKTOPS should be set to 4 as well
- "sanity checks" performed when _NET_DESKTOP_LAYOUT is set but incomplete should be:
 1) applied in all cases, even when _NET_DESKTOP_LAYOUT is not set
 2) the case rows == 0 && columns == 0 should be smarter than just setting the number of rows to 2 and instead compute a number of rows and columns that make the layout be as close as possible to a square (as done in the patch)
- the function is simply too long to be easily understood. Here is a proposal pseudo-code reorganisation:

getWorkspaceCountFromX(workspaceCount)
getWorkspacesLayoutFromX(orientation, columns, rows, startingCorner)

if workspaceCount not set:
    workspaceCount = 4
    setWorkspaceCountToX(workspaceCount)
if orientation not set:
    orientation = horizontal
    XLayoutNeedsUpdate = true
if columns and rows not set:
    columns, rows = values to make up a square
    XLayoutNeedsUpdate = true
else if columns not set:
    columns = ceil(workspaceCount / rows)
    XLayoutNeedsUpdate = true
else if rows not set:
    rows = ceil(workspaceCount / columns)
    XLayoutNeedsUpdate = true
if startingCorner not set:
    startingCorner = topLeft
    XLayoutNeedsUpdate = true

if XLayoutNeedsUpdate:
    setWorkspacesLayoutToX(orientation, columns, rows, startingCorner)

updateLocalProperties(workspaceCount, orientation, columns, rows, startingCorner)

The patch you propose has the following shortcomings of its own:
- the "best approximation for number of rows and cols" is improving on what was done before (just setting the number of rows to 2) but yet duplicating the logic making the code harder to follow
- 'updateX' is set to true when the member variable differs from the local variable (e.g. m_rows != rows); that covers a more cases than it should and we end up resetting _NET_DESKTOP_LAYOUT more than necessary. For example: the very first time WorkspacesInfo::updateWorkspaceGeometry is called by the constructor _NET_DESKTOP_LAYOUT is going to be set even though it might already contain all the right values. _NET_DESKTOP_LAYOUT should only be set when it does not contain one of the values needed to know the layout entirely (e.g. columns is not set). Currently we end up not respecting the layout that an external pa...

[Florian Boucault (fboucault)]

I implemented that vision in https://code.launchpad.net/~fboucault/unity-2d/workspaces_sync_spread_with_wm/+merge/81891

Let me know what you guys think.