The file org_kde_plasma_virtual_desktop.xml should be renamed to virtual-desktops.xml or plasma-virtual-desktops.xml to have a similar name format as the other protocol files we have. Also I would in general prefer to just call it "VirtualDesktop..." instaed of "PlasmaVirtualDesktop..." in the class names (file names alike).

Sometimes it says "desktop" instead of "virtual desktop". Although it's cumbersome I would recommend to always use the term "virtual desktop".

Since virtual desktops form a grid I would have thought first at a 2D matrix / 2D array, i.e.: QVector< QVector< PlasmaVirtualDesktopInterface* > > for storing the global information of all Virtual Desktops and their layout in PlasmaVirtualDesktopManagementInterface. You currently use a QList orderedDesktops for all virtual desktops and "holes" when inserting a virtual desktop in the grid in sortDesktops. This seems a bit clunky and error-prone (in particular the calculations to reorder in sortDesktops).

Now a general thought on the layout mechanism: Currently the number of rows is set and together with the number of virtual desktops this results in a column number. This concept is not straightforward. It is also very limiting in the end: a compositor would not be able to set patterns different from a flow left to right, top to bottom. Even the current KCM is already more involved by offering the option to change row and column count.

I propose the following: Remove the layout_position event from org_kde_plasma_virtual_desktop. Instead let event layout of org_kde_plasma_virtual_desktop_management be: layout(wl_array *virtual_desktops, uint32_t columns) with virtual_desktops representing a matrix with columns many columns of all virtual desktops ids from left to right, top to bottom with 0 in cells where no virtual desktop is placed. This way a compositor can create arbitrary 2D patterns, submit a new matrix to KWayland (KWayland then calculates the wl_array and the columns value) and by that it can change the count and position of virtual desktops atomically when sending the layout event (together with added, remove events if necessary) and the done event afterwards.

so:

• the manager just having a maximum_rows event, which tells that's the boundary of rows the kayout should have

or... not even that (then the setting of maximum row/columns becomes completely kwin internal)

• the location managed completely internally by the virtual desktop so they can have any layout, with holes anywhere
• this simplifies a lot the code protocol-wise (in fact pretty much revertin the last commit in the branch)

but then... how to manage collisions? allowing to set the same row/column to different desktops and make kwin responsibility of this not happening?

In D12820#263516, @mart wrote:

but then... how to manage collisions? allowing to set the same row/column to different desktops and make kwin responsibility of this not happening?

For a layout change the compositor would give KWayland a complete matrix of all virtual desktops indicating their new positions. So KWin would need to make sure that:

• every cell in the matrix only has one VD object referenced (this is per definition if the matrix is QVector<QVector <VirtualDesktopInterface*>> ),
• the matrix contains all available VDs.

The compositor would calculate the new matrix internally (it knows about all available VDs and their current positions through KWayland) and then call:

1. PlasmaVirtualDesktopManagementInterface::layout(QVector<QVector <VirtualDesktopInterface*>> matrix) containing all VDs,
2. sendDone().

On the other side when the compositor wants to create a new VD, it would call:

1. PlasmaVirtualDesktopManagementInterface::createDesktop(),
2. PlasmaVirtualDesktopManagementInterface::layout(QVector<QVector <VirtualDesktopInterface*>> matrix) containing all VDs including the new one,
3. sendDone().

different version for a new round of discussion: try to implement the neighbour concept for the desktop that stay near asnother one, so there may be as many grids unconnected with each other as possible

so far there is not integrity check that an added it is actually an existing desktop, but i would really rather add such checks and auto updates as well.

How is a change of neighbors supposed to work for clients already bound to the proxy objects? For example consider virtual desktop grid:

D1 D2
D3 D4

Compositor removes D3 and let D4 flow back to the left:

D1 D2
D4

desktop_removed event is sent and for D1 and D2 bottom neighbor changes, for D4 left and top neighbor change. To have a consistent state on the client at all time, one would need to tell the client when it has fully received all these information coming from multiple interfaces. Maybe it is enough to send the done event of the management interface not after itself has finished all events, but also every desktop interface has sent it. Not sure if this is fine protocol hygiene though. If it is we need to add it to the descriptions of the interfaces.

General rule of thumb for any data storage for me is to never have a possible corruptable state.
The current protocol which has the position specified inside the Virtual Desktop object itself, so you can have two in the same place and you can't swap the position of two in an atomic move.
This sibling part makes that worse.

I would prefer to not have any layout in the protocol. Setting a layout makes it impossible to have different virtual desktop per output.

Completely agree, but for completely different reasons.

Layout is not a property of the virtual desktop itself but a property of the relevant view.

In the current (even on X state) I can represent 4 virtual desktops in 1 2x2 grid on the pager so it fits best, yet 4x1 along the edges of the cube, and it wouldn't be unfeasible to display them 1x4 in an activity manager style switcher.

What I think would work best is a shared ordered-list, but the visual representation of that ordered list is entirely up to the view.

Kwin still needs to turn that into a grid in some places for keyboard nav and the slide effect, but that's just the case of a single int config value.

In D12820#275739, @davidedmundson wrote:

In the current (even on X state) I can represent 4 virtual desktops in 1 2x2 grid on the pager so it fits best, yet 4x1 along the edges of the cube, and it wouldn't be unfeasible to display them 1x4 in an activity manager style switcher.

What I think would work best is a shared ordered-list, but the visual representation of that ordered list is entirely up to the view.

Kwin still needs to turn that into a grid in some places for keyboard nav and the slide effect, but that's just the case of a single int config value.

so...
Remove all layout position and siblings, and just pay attention
plasmaVirtualDesktops() on server, and
desktops() on the client always have the same order.
Would there be some protocol to reorder desktops or would be just a detail internal in kwin?

wether they are on a line, on a cube, on a 3 rows grid, then would just be a config value setting shared between kwin and the pager, not passing trough wayland in any way.

whith this, would still be possible to allow maybe in the future desktops per-output whicj Martin was talking about?

Don't feel the need to change the code unless you and whoever else agree.
I'm not forcing anything, just commenting on my preference.

But yeah, that would be my proposal.

It'd mean reverting work here, but porting the existing plasma pager/effects would go back to being a lot easier.
I don't even know how I'd go about porting it with this.

Would there be some protocol to reorder desktops or would be just a detail internal in kwin?

I think you'd need to communicate the same order with all clients.
That could be just replacing virtual_desktop_added/virtual_desktop_removed/done with set_virtual_desktops(wl_array<string>); or something else.

whith this, would still be possible to allow maybe in the future desktops per-output whicj Martin was talking about?

WRT positioning, probably. You'd need new API, but you wouldn't have redundant API.
WRT which one is active, this code wouldn't cover that as-is.

In D12820#275870, @davidedmundson wrote:

Don't feel the need to change the code unless you and whoever else agree.
I'm not forcing anything, just commenting on my preference.

nono, i'm just trying to thoroughly think about it :)

I think you'd need to communicate the same order with all clients.
That could be just replacing virtual_desktop_added/virtual_desktop_removed/done with set_virtual_desktops(wl_array<string>); or something else.

that may make more difficult to notice what desktop has been added...
what about virtual_desktop_added*id, position)?
that way, mapping it to a model would become really trivial

whith this, would still be possible to allow maybe in the future desktops per-output whicj Martin was talking about?

WRT positioning, probably. You'd need new API, but you wouldn't have redundant API.
WRT which one is active, this code wouldn't cover that as-is.

it would work if added/removed also had an output as parameters, then they can be different independent sets, tough that would force us to think about it right now, which i'm not too sure

We can remove every layout reference in the manager as well as in the VDs. I don't see the advantage then with a list in contrast to events desktopAdded / desktopRemoved and a done event. What is better about having an ordering? It's only relevant for the special case of having a "flow" of VDs over several lines, assumed we additionally share a single integer with the workspace (via config file or DBus).

But if we decide to not have any layout reference in the VD protocol, then we should also not think about enabling this special case. Instead the VDs should be announced unordered and then a layout is shared with workspace (via config file or DBus).

On the other side VDs without a layout are somewhat futile. Also what we achieve is just deferring the syncing step to the client, which still has to sync the Dbus/config change signal with the Wayland protocol events. One could argue it becomes even worse now over multiple channels.

Still it could make sense because we have a minimal protocol this way as a start and can later extend it if we see problems with the Dbus/config side channel.

What is better about having an ordering?
It's only relevant for the special case of having a "flow" of VDs over several

It's relevant in all cases.

You'd want the context menu of kwin's menu and the task manager to be the same.
Same for any kwin effect that shows it. Whether it's a carousel list, or if it was mapped in a 3D way round all 6 sides of a cube.

I think it counts as a property of the data.

On the other side VDs without a layout are somewhat futile.

Not really. The entire basis of my comments is that we currently commonly lay things out different in different UIs.

Cool. I'm happy with the protocol, except for one important comment.

IMHO worth doing the kwin/plasma code before pushing the frameworks code.

Looking to wire it into KWin:
so, the first thing i note is that abstractclient has already integration api with virtualdesktops with the old assumptions of numerical id and window on either one or all desktops, which is what Client is using on X11.

A virtualdesktopmanager is always created, both in wayland and x11 (in wayland atm to manage xwayland i guess, tough there seems to be partial support for the old virtualdesktop paradigm, both in Shellclient and in the plasmawindow protocol)

What i want to try to do now, is to add usage of the virtual desktop protocol directly in VirtualDesktopManager of virtualdesktops.cpp (all code that would be disable when a waylandserver is not found)

that would be the easiest place to wire up the protocol's virtualdesktopmanagement

The VD protocol was a pure "consumer" protocol before this last revision. Now it gets more complicated by putting requests into it for asking the compositor to change its configuration. I thought this was supposed to go through Dbus. How do you plan to restrict
sandboxed apps in order to not mess with the compositor configuration now?

In D12820#286960, @romangg wrote:

The VD protocol was a pure "consumer" protocol before this last revision. Now it gets more complicated by putting requests into it for asking the compositor to change its configuration. I thought this was supposed to go through Dbus. How do you plan to restrict

sandboxed apps in order to not mess with the compositor configuration now?

It would be a restricted one, usable only by plasma, like plasmashell protocol and such
Tough is a good point... it may make sense to go trough dbus instead (so the KCM can use this as well)
But.. how would you implement "move this window to a new desktop" in that case?

In D12820#286961, @mart wrote:

But.. how would you implement "move this window to a new desktop" in that case?

This functionality is part of the (amended) plasma window management protocol and not of the new VD protocol.

Btw the current diff is somewhat broken. It incorporates besides your changes other changes from the last few weeks.

I have a problem: When I restart plasmashell, I don't get any desktops again. I think the code may not be re-sending the data when the client goes away and comes a second time.

classes renamed to fooUnstableV1, but i need comments, i'm not sure about the naming or how to manage the fact that's unstable..
‎‎Most of the unstable protocols just have the protocol implementation as unstable, but already kinda commit to stability to the c++ interface (as opposed to have a name which has unstable, and an incompatible version would have another class that can be as incompatible as it wants)

‎So, not adjusting the kwin part to it, as i'm not sure at all on what we want yet

RFC: add back some info for the layout? (in particular: rows and orientation, tought orientation is a bit concerning as is view-spacific)

In D12820#297896, @mart wrote:

RFC: add back some info for the layout? (in particular: rows and orientation, tought orientation is a bit concerning as is view-spacific)

it was decided no, we care only about the order