D12820: Add KWayland virtual desktop protocol

This revision was automatically updated to reflect the committed changes. Closed by commit R127:d0db5da57563: Add KWayland virtual desktop protocol (authored by mart). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D12820?vs=38123&id=44643#toc REPOSITORY R127 KWayland CHANGES SINCE LAS

D12820: Add KWayland virtual desktop protocol

davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R127 KWayland BRANCH mart/plasmavirtualdesktop REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #kwin, #plasma, graesslin, hein, davidedmundson Cc: davidedmundson, zzag, bshah, r

D12820: Add KWayland virtual desktop protocol

mart added a comment. 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 or

D12820: Add KWayland virtual desktop protocol

ngraham added a dependent revision: D13887: use the new plasma virtual desktop protocol. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #kwin, #plasma, graesslin, hein Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham,

D12820: Add KWayland virtual desktop protocol

mart added a comment. RFC: add back some info for the layout? (in particular: rows and orientation, tought orientation is a bit concerning as is view-spacific) REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #kwin, #plasma, graesslin, hein Cc: david

D12820: Add KWayland virtual desktop protocol

hein added a task: T4457: [kwayland] Virtual Desktop protocol. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #kwin, #plasma, graesslin, hein Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 38123. mart added a comment. - Revert "rename the virtual desktop to Unstable" - Merge branch 'master' into mart/plasmavirtualdesktop REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12820?vs=38122&id=38123 BRANCH mart

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 38122. mart added a comment. rever rename REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12820?vs=38017&id=38122 BRANCH mart/plasmavirtualdesktop REVISION DETAIL https://phabricator.kde.org/D12820 AFFECTED FILES

D12820: Add KWayland virtual desktop protocol

mart added a comment. 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++ interfa

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 38017. mart added a comment. - fix initial state on createresource - send done to new clients - desktops shouldn't be deleted when the clients go away - iget rid of setActiveDesktop - rename the virtual desktop to Unstable REPOSITORY R127 KWayland

D12820: Add KWayland virtual desktop protocol

hein added a comment. 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. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 37197. mart added a comment. - ensure there are never duplicates REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12820?vs=37149&id=37197 BRANCH mart/plasmavirtualdesktop REVISION DETAIL https://phabricator.kde.org/D1

D12820: Add KWayland virtual desktop protocol

romangg added a comment. 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 V

D12820: Add KWayland virtual desktop protocol

mart added a comment. 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.

D12820: Add KWayland virtual desktop protocol

romangg added a comment. 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 s

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 37149. mart added a comment. - add a request remove to the protocol - add api call to request removal of a desktop - requestCreate - remove window management features: - active() -> isactive() - requestEnterNewVirtualDesktop REPOSITORY R127 KWayla

D12820: Add KWayland virtual desktop protocol

mart marked an inline comment as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #kwin, #plasma, graesslin, hein Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 36888. mart marked an inline comment as done. mart added a comment. - better comments REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12820?vs=36887&id=36888 BRANCH mart/plasmavirtualdesktop REVISION DETAIL https://p

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 36887. mart added a comment. - add desktopActivated signal - correct behavior for set on all desktops - add new tests for on all desktops - make the old isOnallVirtualDesktops api work REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phab

D12820: Add KWayland virtual desktop protocol

hein added a dependent revision: D13745: Implement support for virtual desktops on Wayland. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #kwin, #plasma, graesslin, hein Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraha

D12820: Add KWayland virtual desktop protocol

romangg added inline comments. INLINE COMMENTS > plasma-virtual-desktop.xml:55 > + > + > + This needs more description. Other events below as well. > plasmavirtualdesktop_interface.cpp:181 > + > +PlasmaVirtualDesktopInterface > *PlasmaVirtualDesktopManagementInterface::createDe

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 36528. mart added a comment. - Merge branch 'master' into mart/plasmavirtualdesktop REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12820?vs=36161&id=36528 BRANCH mart/plasmavirtualdesktop REVISION DETAIL https://pha

D12820: Add KWayland virtual desktop protocol

mart added a comment. 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 virtualdeskt

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 36161. mart added a comment. - remove release from the protocol - new_id is id REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12820?vs=36052&id=36161 BRANCH mart/plasmavirtualdesktop REVISION DETAIL https://phabri

D12820: Add KWayland virtual desktop protocol

davidedmundson added a comment. Cool. I'm happy with the protocol, except for one important comment. IMHO worth doing the kwin/plasma code before pushing the frameworks code. INLINE COMMENTS > plasma-virtual-desktop.xml:62 > + > + > + This still isn't deletable, see my comment f

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 36052. mart added a comment. - remove neighbours, only order - store data in a single structure - auto activate a new desktop REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12820?vs=34871&id=36052 BRANCH mart/plasm

D12820: Add KWayland virtual desktop protocol

davidedmundson added a comment. > 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 kw

D12820: Add KWayland virtual desktop protocol

romangg added a comment. 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 cas

D12820: Add KWayland virtual desktop protocol

mart added a comment. 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 thorough

D12820: Add KWayland virtual desktop protocol

davidedmundson added inline comments. INLINE COMMENTS > plasma-virtual-desktop.xml:56 > + > + > + Code comment: This interface doesn't have a destructor. The server can invalidate it on it's end, but ultimately up to the client to destroy the reference which they can't do. REPOSITORY R

D12820: Add KWayland virtual desktop protocol

davidedmundson added a comment. 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 wo

D12820: Add KWayland virtual desktop protocol

mart added a comment. 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

D12820: Add KWayland virtual desktop protocol

davidedmundson added a comment. 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 pos

D12820: Add KWayland virtual desktop protocol

romangg added a comment. 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` e

D12820: Add KWayland virtual desktop protocol

mart added a comment. 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 actuall

D12820: Add KWayland virtual desktop protocol

mart updated this revision to Diff 34871. mart added a comment. - Revert "virtual diland Virtual desktop protocol" - rename the xml file to be consistent with the rest - fix copyright - replace the concept of layout with neighbours - smaller fixes REPOSITORY R127 KWayland CHANGES SI

D12820: Add KWayland virtual desktop protocol

mart added inline comments. INLINE COMMENTS > graesslin wrote in org_kde_plasma_virtual_desktop.xml:45 > Please note that your quick way would not work. Activities are disabled in > KWin Wayland and I don't see them getting enabled again. Activities resulted > in hard freezes several times and

D12820: Add KWayland virtual desktop protocol

graesslin added inline comments. INLINE COMMENTS > mart wrote in org_kde_plasma_virtual_desktop.xml:45 > the virtualdesktop would then have an output property used for filtering? > or, the layout event could be kept in the manager, but be not just rows and > column, but pass the pair output and

D12820: Add KWayland virtual desktop protocol

mart added inline comments. INLINE COMMENTS > graesslin wrote in org_kde_plasma_virtual_desktop.xml:45 > Have a look at i3. Btw it's a really often requested feature. the virtualdesktop would then have an output property used for filtering? or, the layout event could be kept in the manager, but

D12820: Add KWayland virtual desktop protocol

graesslin added inline comments. INLINE COMMENTS > mart wrote in org_kde_plasma_virtual_desktop.xml:45 > How you would envision different virtual desktops per output? > to me is a bit of a weird concept, as an output is an actual desktop, not > virtual? Have a look at i3. Btw it's a really ofte

D12820: Add KWayland virtual desktop protocol

zzag added inline comments. INLINE COMMENTS > plasmavirtualdesktop_interface.cpp:36 > + > +class PlasmaVirtualDesktopInterface::Private > +{ class Q_DECL_HIDDEN PlasmaVirtualDesktopInterface::Private Same for the server part. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kd

D12820: Add KWayland virtual desktop protocol

mart added inline comments. INLINE COMMENTS > graesslin wrote in org_kde_plasma_virtual_desktop.xml:45 > A possibility could be a sibling concept. Every desktop could have a > reference to the desktop to the left/top/right/bottom. How you would envision different virtual desktops per output? to

D12820: Add KWayland virtual desktop protocol

graesslin added inline comments. INLINE COMMENTS > mart wrote in org_kde_plasma_virtual_desktop.xml:45 > Ok, so no layout information at all neither in the manager nor in the single > virtual desktops? > I kindof like that as makes things a lot simpler. Tough in this case if the > concept of la

D12820: Add KWayland virtual desktop protocol

mart added inline comments. INLINE COMMENTS > graesslin wrote in org_kde_plasma_virtual_desktop.xml:45 > I would prefer to not have any layout in the protocol. Setting a layout makes > it impossible to have different virtual desktop per output. Ok, so no layout information at all neither in the

D12820: Add KWayland virtual desktop protocol

graesslin added inline comments. INLINE COMMENTS > org_kde_plasma_virtual_desktop.xml:4 > +

D12820: Add KWayland virtual desktop protocol

romangg added a comment. 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 composi

D12820: Add KWayland virtual desktop protocol

mart added a comment. 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 b

D12820: Add KWayland virtual desktop protocol

romangg added inline comments. INLINE COMMENTS > mart wrote in org_kde_plasma_virtual_desktop.xml:45 > isn't better the layout to be communicated by the manager? the reasoning was > to have it loosely map on models. > the model tells how many rows it has, then the single item on what row it is

D12820: Add KWayland virtual desktop protocol

mart added inline comments. INLINE COMMENTS > bshah wrote in registry.h:154 > This should be added at end and not in middle and also add `@since 5.47(?)` you're right this breaks ABI REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #kwin, #plasma, grae

D12820: Add KWayland virtual desktop protocol

bshah added inline comments. INLINE COMMENTS > registry.h:154 > PlasmaShell, ///< Refers to org_kde_plasma_shell interface > +PlasmaVirtualDesktopManagement, ///< Refers to > org_kde_plasma_virtual_desktop_management interface > PlasmaWindowManagement, ///< Refers to >

D12820: Add KWayland virtual desktop protocol

mart added inline comments. INLINE COMMENTS > romangg wrote in org_kde_plasma_virtual_desktop.xml:45 > This event is redundant with the current implementation. A client can infer > the number of rows and columns from the org_kde_plasma_virtual_desktop > objects it receives and their maximum row

D12820: Add KWayland virtual desktop protocol

romangg added a comment. 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 "Plas

D12820: Add KWayland virtual desktop protocol

mart added a comment. An important part the review should adress is how compatible the current api is to the current KWin architecture: the virtualdesktopmamagement interface is made to correspond pretty much to the new virtualDesktop class, tough both Workspace and AbstractClient still see

D12820: Add KWayland virtual desktop protocol

mart added a comment. A part i'm not sure is the amount of behavior and consistency checks in the server part: right now it renumbers the virtual desktops in function of the grid and tries to keep always one desktop active. should this be done (also) in the kwayland protocol server part o

D12820: Add KWayland virtual desktop protocol

mart added a comment. Only difference is the plasmawindow part api to have just the ids for consistence (always easy to get to the objects anyways) REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #kwin, #plasma, graesslin, hein Cc: kde-frameworks

D12820: Add KWayland virtual desktop protocol

mart retitled this revision from "virtual diland Virtual desktop protocol" to "Add KWayland virtual desktop protocol". mart added reviewers: graesslin, hein. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #kwin, #plasma, graesslin, hein Cc: kde-framewo