D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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=44643#toc

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12820?vs=38123=44643

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  autotests/client/CMakeLists.txt
  autotests/client/test_plasma_virtual_desktop.cpp
  src/client/CMakeLists.txt
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/client/registry.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/tools/mapping.txt

To: mart, #kwin, #plasma, graesslin, hein, davidedmundson
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[David Edmundson]

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, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 order

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

[Nathaniel Graham]

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, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Eike Hein]

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

[Marco Martin]

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=38123

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  autotests/client/CMakeLists.txt
  autotests/client/test_plasma_virtual_desktop.cpp
  src/client/CMakeLists.txt
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/client/registry.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/tools/mapping.txt

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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=38122

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  .arclint
  CMakeLists.txt
  autotests/client/CMakeLists.txt
  autotests/client/test_datadevice.cpp
  autotests/client/test_plasma_virtual_desktop.cpp
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_plasmashell.cpp
  autotests/client/test_wayland_registry.cpp
  autotests/client/test_wayland_surface.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  autotests/client/test_xdg_output.cpp
  autotests/client/test_xdg_shell_v6.cpp
  src/client/CMakeLists.txt
  src/client/plasmashell.cpp
  src/client/plasmashell.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/fullscreen-shell.xml
  src/client/protocols/outputdevice.xml
  src/client/protocols/plasma-shell.xml
  src/client/protocols/plasma-window-management.xml
  src/client/protocols/xdg-output-unstable-v1.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/textinput.h
  src/client/textinput_v0.cpp
  src/client/textinput_v2.cpp
  src/client/xdgoutput.cpp
  src/client/xdgoutput.h
  src/client/xdgshell_v6.cpp
  src/server/CMakeLists.txt
  src/server/datadevice_interface.cpp
  src/server/display.cpp
  src/server/display.h
  src/server/plasmashell_interface.cpp
  src/server/plasmashell_interface.h
  src/server/plasmavirtualdesktop_interface_unstable_v1.cpp
  src/server/plasmavirtualdesktop_interface_unstable_v1.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/server/surface_interface.cpp
  src/server/xdgforeign_v2_interface.cpp
  src/server/xdgoutput_interface.cpp
  src/server/xdgoutput_interface.h
  src/tools/mapping.txt
  tests/plasmasurfacetest.cpp

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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++ 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

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

[Marco Martin]

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

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12820?vs=37197=38017

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  .arclint
  CMakeLists.txt
  autotests/client/CMakeLists.txt
  autotests/client/test_datadevice.cpp
  autotests/client/test_plasma_virtual_desktop.cpp
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_plasmashell.cpp
  autotests/client/test_wayland_registry.cpp
  autotests/client/test_wayland_surface.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  autotests/client/test_xdg_output.cpp
  autotests/client/test_xdg_shell_v6.cpp
  src/client/CMakeLists.txt
  src/client/plasmashell.cpp
  src/client/plasmashell.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/fullscreen-shell.xml
  src/client/protocols/outputdevice.xml
  src/client/protocols/plasma-shell.xml
  src/client/protocols/plasma-window-management.xml
  src/client/protocols/xdg-output-unstable-v1.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/textinput.h
  src/client/textinput_v0.cpp
  src/client/textinput_v2.cpp
  src/client/xdgoutput.cpp
  src/client/xdgoutput.h
  src/client/xdgshell_v6.cpp
  src/server/CMakeLists.txt
  src/server/datadevice_interface.cpp
  src/server/display.cpp
  src/server/display.h
  src/server/plasmashell_interface.cpp
  src/server/plasmashell_interface.h
  src/server/plasmavirtualdesktop_interface_unstable_v1.cpp
  src/server/plasmavirtualdesktop_interface_unstable_v1.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/server/surface_interface.cpp
  src/server/xdgforeign_v2_interface.cpp
  src/server/xdgoutput_interface.cpp
  src/server/xdgoutput_interface.h
  src/tools/mapping.txt
  tests/plasmasurfacetest.cpp

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Eike Hein]

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, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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=37197

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  .arclint
  CMakeLists.txt
  autotests/client/CMakeLists.txt
  autotests/client/test_datadevice.cpp
  autotests/client/test_plasma_virtual_desktop.cpp
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_plasmashell.cpp
  autotests/client/test_wayland_registry.cpp
  autotests/client/test_wayland_surface.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  autotests/client/test_xdg_output.cpp
  autotests/client/test_xdg_shell_v6.cpp
  src/client/CMakeLists.txt
  src/client/plasmashell.cpp
  src/client/plasmashell.h
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/fullscreen-shell.xml
  src/client/protocols/outputdevice.xml
  src/client/protocols/plasma-shell.xml
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/protocols/xdg-output-unstable-v1.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/textinput.h
  src/client/textinput_v0.cpp
  src/client/textinput_v2.cpp
  src/client/xdgoutput.cpp
  src/client/xdgoutput.h
  src/client/xdgshell_v6.cpp
  src/server/CMakeLists.txt
  src/server/datadevice_interface.cpp
  src/server/display.cpp
  src/server/display.h
  src/server/plasmashell_interface.cpp
  src/server/plasmashell_interface.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/server/surface_interface.cpp
  src/server/xdgforeign_v2_interface.cpp
  src/server/xdgoutput_interface.cpp
  src/server/xdgoutput_interface.h
  src/tools/mapping.txt
  tests/plasmasurfacetest.cpp

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Roman Gilg]

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 VD protocol.
  
  Btw the current diff is somewhat broken. It incorporates besides your changes 
other changes from the last few weeks.

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

[Marco Martin]

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. 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?

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

[Roman Gilg]

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 
   sandboxed apps in order to not mess with the compositor configuration now?

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

[Marco Martin]

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 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12820?vs=36888=37149

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  .arclint
  CMakeLists.txt
  autotests/client/CMakeLists.txt
  autotests/client/test_datadevice.cpp
  autotests/client/test_plasma_virtual_desktop.cpp
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_plasmashell.cpp
  autotests/client/test_wayland_registry.cpp
  autotests/client/test_wayland_surface.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  autotests/client/test_xdg_output.cpp
  autotests/client/test_xdg_shell_v6.cpp
  src/client/CMakeLists.txt
  src/client/plasmashell.cpp
  src/client/plasmashell.h
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/fullscreen-shell.xml
  src/client/protocols/outputdevice.xml
  src/client/protocols/plasma-shell.xml
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/protocols/xdg-output-unstable-v1.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/textinput.h
  src/client/textinput_v0.cpp
  src/client/textinput_v2.cpp
  src/client/xdgoutput.cpp
  src/client/xdgoutput.h
  src/client/xdgshell_v6.cpp
  src/server/CMakeLists.txt
  src/server/datadevice_interface.cpp
  src/server/display.cpp
  src/server/display.h
  src/server/plasmashell_interface.cpp
  src/server/plasmashell_interface.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/server/surface_interface.cpp
  src/server/xdgforeign_v2_interface.cpp
  src/server/xdgoutput_interface.cpp
  src/server/xdgoutput_interface.h
  src/tools/mapping.txt
  tests/plasmasurfacetest.cpp

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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

[Marco Martin]

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=36888

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  .arclint
  CMakeLists.txt
  autotests/client/CMakeLists.txt
  autotests/client/test_datadevice.cpp
  autotests/client/test_plasma_virtual_desktop.cpp
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_plasmashell.cpp
  autotests/client/test_wayland_registry.cpp
  autotests/client/test_wayland_surface.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  autotests/client/test_xdg_output.cpp
  autotests/client/test_xdg_shell_v6.cpp
  src/client/CMakeLists.txt
  src/client/plasmashell.cpp
  src/client/plasmashell.h
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/fullscreen-shell.xml
  src/client/protocols/outputdevice.xml
  src/client/protocols/plasma-shell.xml
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/protocols/xdg-output-unstable-v1.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/textinput.h
  src/client/textinput_v0.cpp
  src/client/textinput_v2.cpp
  src/client/xdgoutput.cpp
  src/client/xdgoutput.h
  src/client/xdgshell_v6.cpp
  src/server/CMakeLists.txt
  src/server/datadevice_interface.cpp
  src/server/display.cpp
  src/server/display.h
  src/server/plasmashell_interface.cpp
  src/server/plasmashell_interface.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/server/surface_interface.cpp
  src/server/xdgforeign_v2_interface.cpp
  src/server/xdgoutput_interface.cpp
  src/server/xdgoutput_interface.h
  src/tools/mapping.txt
  tests/plasmasurfacetest.cpp

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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://phabricator.kde.org/D12820?vs=36528=36887

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  .arclint
  CMakeLists.txt
  autotests/client/CMakeLists.txt
  autotests/client/test_datadevice.cpp
  autotests/client/test_plasma_virtual_desktop.cpp
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_plasmashell.cpp
  autotests/client/test_wayland_registry.cpp
  autotests/client/test_wayland_surface.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  autotests/client/test_xdg_output.cpp
  autotests/client/test_xdg_shell_v6.cpp
  src/client/CMakeLists.txt
  src/client/plasmashell.cpp
  src/client/plasmashell.h
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/fullscreen-shell.xml
  src/client/protocols/outputdevice.xml
  src/client/protocols/plasma-shell.xml
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/protocols/xdg-output-unstable-v1.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/textinput.h
  src/client/textinput_v0.cpp
  src/client/textinput_v2.cpp
  src/client/xdgoutput.cpp
  src/client/xdgoutput.h
  src/client/xdgshell_v6.cpp
  src/server/CMakeLists.txt
  src/server/datadevice_interface.cpp
  src/server/display.cpp
  src/server/display.h
  src/server/plasmashell_interface.cpp
  src/server/plasmashell_interface.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/server/surface_interface.cpp
  src/server/xdgforeign_v2_interface.cpp
  src/server/xdgoutput_interface.cpp
  src/server/xdgoutput_interface.h
  src/tools/mapping.txt
  tests/plasmasurfacetest.cpp

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Eike Hein]

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, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Roman Gilg]

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::createDesktop(const QString )
> +{

What happens on an empty QString argument? I wouldn't allow it and abort with 
nullptr returned directly.

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

[Marco Martin]

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=36528

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  autotests/client/CMakeLists.txt
  autotests/client/test_plasma_virtual_desktop.cpp
  src/client/CMakeLists.txt
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/client/registry.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/tools/mapping.txt

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 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

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

[Marco Martin]

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=36161

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  autotests/client/CMakeLists.txt
  autotests/client/test_plasma_virtual_desktop.cpp
  src/client/CMakeLists.txt
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/client/registry.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/tools/mapping.txt

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[David Edmundson]

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 from last time.

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

[Marco Martin]

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=36052

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  autotests/client/CMakeLists.txt
  autotests/client/test_plasma_virtual_desktop.cpp
  src/client/CMakeLists.txt
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/client/registry.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/tools/mapping.txt

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[David Edmundson]

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 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.

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

[Roman Gilg]

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 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.

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

[Marco Martin]

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 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);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

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

[David Edmundson]

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
  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

[David Edmundson]

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 
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);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.

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

[Marco Martin]

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 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?

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

[David Edmundson]

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 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.

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

[Roman Gilg]

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` 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.

INLINE COMMENTS

> plasma-virtual-desktop.xml:25
> +
> + interface="org_kde_plasma_virtual_desktop"/>
> +

This arg name should be `id` (judging from other example protocols). Find a 
different arg name for the second argument, for example `desktop_id` or just 
`desktop`. Together:

  
  

> mart wrote in plasma-window-management.xml:273
> tough is a thing the client asks for which may or may not be granted.. just 
> enter_virtual_desktop looks like it's already decided?

It's a request, so it's intrinsic that it's not yet decided. You can say in the 
description, that the server might ignore this request, otherwise it sends the 
virtual_desktop_entered event.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 actually an existing 
desktop, but i would really rather add such checks and auto updates as well.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 SINCE LAST UPDATE
  https://phabricator.kde.org/D12820?vs=33979=34871

BRANCH
  mart/plasmavirtualdesktop

REVISION DETAIL
  https://phabricator.kde.org/D12820

AFFECTED FILES
  autotests/client/CMakeLists.txt
  autotests/client/test_plasma_virtual_desktop.cpp
  src/client/CMakeLists.txt
  src/client/plasmavirtualdesktop.cpp
  src/client/plasmavirtualdesktop.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-virtual-desktop.xml
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/client/registry.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/plasmavirtualdesktop_interface.cpp
  src/server/plasmavirtualdesktop_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  src/tools/mapping.txt

To: mart, #kwin, #plasma, graesslin, hein
Cc: zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 I don't trust them due to that. As 
> maintainer I would need very convincing arguments on why activities won't 
> break KWin again. Due to the fact that kamd speaks both dbus and windowing 
> protocol I don't see a chance that KWin can rely on it.
> 
> Due to the fact that activities used to block KWin it's an area which is 
> completely not ported to Wayland yet. A Wayland window cannot be put on an 
> activity. So this would require a significant amount of work. This also 
> doesn't match anything we have in KWin to move forward. Activities have been 
> in an unmaintained state since the start and the code is in a really bad 
> state. None of the core KWin developers have touched it in the last year's. 
> It's just a bad copy and paste of virtual desktops without any understanding 
> of the domain. I do not want to see any future code being based on that. In 
> KWin we started to evolve virtual desktops, so that in future it would be 
> possible to implement the feature set of activities in virtual desktops. But 
> for the time being it would mean X11 identifiers, that is numerical desktops.
> 
> I put know out an interesting question: do we need layouts?

well, i'm not really talking about the current activity code in kwin, but just 
a 1:1 mapping for the user, for which i was expecting different code to be 
written.
if the 1:1 mapping happens on kwin side, or on plasma/kactivities side trying 
to sync itself to desktops (in this case the protocol would need also 
adddesktop/removedesktop methods, then kwin wouldn't need to use kactivities 
directly, the sync can be done client side.

but i don't want the user to see any difference anymore for the two concepts 
anymore.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Martin Flöser]

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 actual vector containing the 
> virtualdesktop instances?
> 
> also if it is per output then two desktops can be on the same row/column, as 
> long as they are on different outputs..
> then also all the api for requesting/notifying activate/deactivate would need 
> an output parameter.. /o\
> 
> My primary use for this protocol was to make activities==virtual desktops 
> (the quick way would be to just use the activity ids as desktop ids and use 
> kactivitymanager to add/remove, so no need for api to ask kwin to add/remove 
> desktops)
> this clashes a bit with different desktops per output, tough could be solved 
> if i then make the activity id a separate property in the Virtualdesktop 
> interface...

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 I don't trust them due to that. As maintainer I would 
need very convincing arguments on why activities won't break KWin again. Due to 
the fact that kamd speaks both dbus and windowing protocol I don't see a chance 
that KWin can rely on it.

Due to the fact that activities used to block KWin it's an area which is 
completely not ported to Wayland yet. A Wayland window cannot be put on an 
activity. So this would require a significant amount of work. This also doesn't 
match anything we have in KWin to move forward. Activities have been in an 
unmaintained state since the start and the code is in a really bad state. None 
of the core KWin developers have touched it in the last year's. It's just a bad 
copy and paste of virtual desktops without any understanding of the domain. I 
do not want to see any future code being based on that. In KWin we started to 
evolve virtual desktops, so that in future it would be possible to implement 
the feature set of activities in virtual desktops. But for the time being it 
would mean X11 identifiers, that is numerical desktops.

I put know out an interesting question: do we need layouts?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 be not just rows and 
column, but pass the pair output and actual vector containing the 
virtualdesktop instances?

also if it is per output then two desktops can be on the same row/column, as 
long as they are on different outputs..
then also all the api for requesting/notifying activate/deactivate would need 
an output parameter.. /o\

My primary use for this protocol was to make activities==virtual desktops (the 
quick way would be to just use the activity ids as desktop ids and use 
kactivitymanager to add/remove, so no need for api to ask kwin to add/remove 
desktops)
this clashes a bit with different desktops per output, tough could be solved if 
i then make the activity id a separate property in the Virtualdesktop 
interface...

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Martin Flöser]

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 often requested feature.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Vlad Zagorodniy]

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.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: zzag, bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 me is a bit of a weird concept, as an output is an actual desktop, not 
virtual?

> 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.

> A possibility could be a sibling concept. Every desktop could have a 
> reference to the desktop to the left/top/right/bottom.

wouldn't become harder to track and rebuild a visual representation from it?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Martin Flöser]

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 layout becomes completely internal in kwin, how to communicate it 
> to the client? a pager would still need to loosely represent the same layout.

A possibility could be a sibling concept. Every desktop could have a reference 
to the desktop to the left/top/right/bottom.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 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 layout becomes completely internal in kwin, how to communicate it to 
the client? a pager would still need to loosely represent the same layout.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Martin Flöser]

graesslin added inline comments.

INLINE COMMENTS

> org_kde_plasma_virtual_desktop.xml:4
> +  

D12820: Add KWayland virtual desktop protocol

[Roman Gilg]

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 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>` ),
  - 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> 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> matrix)` containing all VDs including the new one,
  3. `sendDone()`.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 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?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Roman Gilg]

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 
> located.
> 
> Also, I think is to be decided now as it influences everything else:
> Do we want to allow "holes" in the order? (as you said one in first row, 3 in 
> second etc)

> isn't better the layout to be communicated by the manager?

In the review comment with the "different approach" I propose exactly this. 
I.e. not remove this (currently redundant) event `layout`, but even promote it 
by not only sending the current row and column count, but also directly all 
positions of existing VDs through a wl_array.

> 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 
> located.

Thanks, good to know the idea behind it. I see mostly a problem with 
synchronization between the single virtual desktop objects among themselves as 
well as with the manager with this approach. For example if the compositor 
decides to remove a virtual desktop, what reduces the row/column count: Is the 
`layout` event first emitted or the `layout_position` events for the remaining 
VDs? How in both cases should the client react?

If the client first receives the `layout` event it still does not really know 
which VD has been removed. If it first receives all `layout_position` events 
plus the `removed` event it does not need the `layout` event any more.

But in any case the client would need to hold the VDs with already received 
`layout_position` events in some intermediate state until it has received the 
events for all VDs plus the removed event for the VD, which got removed. 
Otherwise it might put multiple VDs on the same position.

> Do we want to allow "holes" in the order? (as you said one in first row, 3 in 
> second etc)

For this protocol extension I would vote in favor of that, because there are 
valid use cases and we should write our protocols directly for a larger 
audience if possible. Here if we use a matrix for representation (with 0 if 
there is a hole) it wouldn't introduce much more complexity.

Currently on X we only allow full rows and columns I believe, so the protocol 
implementation in KWin / Plasma could still only use full rows / columns for 
now.

In general in the long run I imagine a KCM, where a user sees the current VDs 
in an overview like with the pager and then has buttons around it with plus 
signs on it the user can click to add another VD in this location. Also with 
this approach we could later on add "spans" over multiple columns/rows. For 
example a main VD and below three auxiliary VDs.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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, graesslin, hein
Cc: bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Bhushan Shah]

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 
> org_kde_plasma_window_management interface

This should be added at end and not in middle and also add `@since 5.47(?)`

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: bshah, romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 and column values.
> 
> Also a compositor might be interested in setting a custom layout like this: 5 
> virtual desktops as a cross (one top, 3 in a row mid, one bottom). While one 
> can still say this has in some sense 3 rows and 3 columns, the event in this 
> case loses its intended meaning of defining the "layout".
> 
> Look at my review comment for a different approach.

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 
located.

Also, I think is to be decided now as it influences everything else:
Do we want to allow "holes" in the order? (as you said one in first row, 3 in 
second etc)

> romangg wrote in plasma-window-management.xml:273
> Just name it `enter_virtual_desktop` I would say. For 
> `request_leave_virtual_desktop` the same.

tough is a thing the client asks for which may or may not be granted.. just 
enter_virtual_desktop looks like it's already decided?

> romangg wrote in plasmavirtualdesktop_interface.h:77
> The compositor does not need to decide upon an id. Let KWayland do this 
> internally (this also means that whenever `createDesktop` is called it is 
> guaranteed that a new PlasmaVirtualDesktopInterface will be returned.
> 
> Or is this needed in regards to Activities down the road? If yes I would also 
> argue that a KWayland internal id would be nicer and KWin should map them 
> then to Activity ids.

what i would like to is to have in the end the ids coming from kactivities, (as 
i want in wayland desktops===activities) which makes more sense to link and use 
in kwin, rather than in kwayland

> romangg wrote in plasmavirtualdesktop_interface.h:103
> That the management class is friend to the things it manages (here the 
> PlasmaVirtualDesktopInterface) ok, but the other way around... I would maybe 
> try to work without it.

yeah, i would need to make things work without needing that ugly ordereddesktops

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: romangg, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Roman Gilg]

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 "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 and 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.

INLINE COMMENTS

> test_plasma_virtual_desktop.cpp:3
> +Copyright 2014  Martin Gräßlin 
> +Copyright 2018  Msrco Martin 
> +

M__a__rco

> plasmavirtualdesktop.h:141
> +Q_SIGNALS:
> +void removed();
> +

This signal is unneeded when `desktopRemoved` is there as well.

> org_kde_plasma_virtual_desktop.xml:5
> +Copyright (C) 2015 Martin Gräßlin
> +Copyright (C) 2015 Marco Martin
> +

2018 and only you Marco?

> org_kde_plasma_virtual_desktop.xml:26
> +
> + interface="org_kde_plasma_virtual_desktop"/>
> +

`name="id"`

> 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 and column values.

Also a compositor might be interested in setting a custom layout like this: 5 
virtual desktops as a cross (one top, 3 in a row mid, one bottom). While one 
can still say this has in some sense 3 rows and 3 columns, the event in this 
case loses its intended meaning of defining the "layout".

Look at my review comment for a different approach.

> org_kde_plasma_virtual_desktop.xml:83
> +
> +Logical position of the visual model of this desktop in terms of 
> rows and columns: there will be only one desktop on a (row, column)
> +

Mention that this event is send on bind and when the position changes.

+ line breaks

> plasma-window-management.xml:273
> +
> +
> +  

Just name it `enter_virtual_desktop` I would say. For 
`request_leave_virtual_desktop` the same.

> plasmavirtualdesktop_interface.cpp:143
> +}
> +});
> +

indent (below as well)

> plasmavirtualdesktop_interface.cpp:200
> +org_kde_plasma_virtual_desktop_management_send_layout(resource, rows, 
> columns);
> +}
> +

Send done event in the end and flush the client.

> plasmavirtualdesktop_interface.h:77
> + */
> +PlasmaVirtualDesktopInterface *createDesktop(const QString );
> +

The compositor does not need to decide upon an id. Let KWayland do this 
internally (this also means that whenever `createDesktop` is called it is 
guaranteed that a new PlasmaVirtualDesktopInterface will be returned.

Or is this needed in regards to Activities down the road? If yes I would also 
argue that a KWayland internal id would be nicer and KWin should map them then 
to Activity ids.

> plasmavirtualdesktop_interface.h:103
> +explicit PlasmaVirtualDesktopManagementInterface(Display *display, 
> QObject *parent = nullptr);
> +friend class PlasmaVirtualDesktopInterface;
> +friend class Display;

That the management class is friend to the things it manages (here the 
PlasmaVirtualDesktopInterface) ok, but the other way around... I 

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 seem  to assume a window can have a single 
desktop.
  On kwin side, i would take the route of completely kactivities-based.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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 or should
  it be completely responsibility of the compositor?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D12820

To: mart, #kwin, #plasma, graesslin, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Marco Martin]

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-frameworks-devel, michaelh, ngraham, bruns