Implement support for virtual desktops on Wayland
ClosedPublic

Authored by hein on Jun 26 2018, 9:30 PM.

Details

Summary

Includes, among other things:

  • A refactoring towards supporting more than one desktop per window, for an eventual virtual desktops / activities merge
  • A scheme for process-internal window ids on Wayland as DND payload so DND in and to the Pager works in the shell
  • Implemented various previously missing behavior in WaylandTasksModel such as implicit moves of windows to the current desktop on various actions
  • Expanded VirtualDesktopInfo API so the Pager can better abstract over windowing systems
  • Implicit internal sharing of VirtualDesktopInfo since there are many more instances now
  • Various cleanups

Still missing:

  • Fixing the VirtualDesktops data role in the grouping proxy
  • The protocol doesn't have desktop creation/destruction yet, so some of the related logic is still missing
  • Some FIXME TODOs in the code when I was unhappy with the current KWayland API

This code is largely untested and subject to change.

Depends on D12820 and relates to T4457.

Diff Detail

Repository
R120 Plasma Workspace
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 626
Build 638: arc lint + arc unit
hein created this revision.Jun 26 2018, 9:30 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJun 26 2018, 9:30 PM
hein requested review of this revision.Jun 26 2018, 9:30 PM
hein planned changes to this revision.Jun 26 2018, 10:11 PM
hein added a comment.Jun 27 2018, 6:25 PM

FWIW: I want to continue with this for now because under the current usage it's still better for performance and lower risk, but longer-term I might refactor VirtualDesktopInfo and ActivityInfo into {AbstractWorkspaceModel,VirtualDesktopModel,ActivityModel} and replace PagerModel directly. It would make for a more consistent lib API and simplify applet code.

hein updated this revision to Diff 36857.Jun 28 2018, 8:05 PM
  • Make WaylandTasksModel::requestVirtualDesktops treat the passed-in list as exhaustive.
  • Fix how an empty list is treated.
mart added inline comments.Jun 29 2018, 1:44 PM
libtaskmanager/waylandtasksmodel.cpp
449

note that the protocol ensures the list of desktops is empty when is on all virtual desktops, even when every desktop has been added by hand

hein updated this revision to Diff 36903.Jun 29 2018, 7:47 PM

Adjust to new semantics for IsOnAllDesktops.

Michail: Please follow this review, as these API changes might impact Latte Dock, too. :)

In D13745#286491, @hein wrote:

Michail: Please follow this review, as these API changes might impact Latte Dock, too. :)

no prob... this will be a big change so it will break Latte.

Latte v0.8 is a very heavy Activities user so only thing can done is when the VDs-Activities architecture lands in Plasma to try to catch up until the official release with a secondary official release from Latte.

If I have understood correctly:

  1. at some plasma point release (e.g. 5.x ) in the feature Activities will be merged with Virtual Desktops both for X11 and wayland?
  2. that will probably break Activties from (5.x-1) ?
  3. The idea is that Virtual Desktops will get some characteristics from Activities or vice versa?
  4. The features provided from kactivitymanagerd will be dropped? (files linked to Activities, Activity icon, etc...) OR Virtual Desktops in Plasma will be identified through kactivitymanagerd in the future?
hein added a comment.Jul 3 2018, 2:09 PM

A lot of this is still up in the air, but here's the rough plan:

0. X11 will remain unchanged. The feature set of Activities will remain unchanged.

  1. For 5.14, we want to have virtual desktops working on Wayland, with feature parity to X11. Activities will not yet work on Wayland. The new Wayland protocol allows windows to be on more than one desktop and it allows for multiple concurrently active desktops, but these abilities will not be used in 5.14, they will only be present in the API (hence the API changes).
  1. For 5.15, we want to sync Activities to virtual desktops on Wayland, meaning adding/removing Activities is how you add/remove virtual desktops, and switching/activating VDs==switching/activating Activities.

That means on Wayland on 5.15+ (if this schedule holds) the default Task Manager would just hide all the virtual desktop UI, exposing only the activities UI.

In D13745#286501, @hein wrote:

A lot of this is still up in the air, but here's the rough plan:

0. X11 will remain unchanged. The feature set of Activities will remain unchanged.

  1. For 5.14, we want to have virtual desktops working on Wayland, with feature parity to X11. Activities will not yet work on Wayland. The new Wayland protocol allows windows to be on more than one desktop and it allows for multiple concurrently active desktops, but these abilities will not be used in 5.14, they will only be present in the API (hence the API changes).
  2. For 5.15, we want to sync Activities to virtual desktops on Wayland, meaning adding/removing Activities is how you add/remove virtual desktops, and switching/activating VDs==switching/activating Activities.

    That means on Wayland on 5.15+ (if this schedule holds) the default Task Manager would just hide all the virtual desktop UI, exposing only the activities UI.

I see...

The thing is that I can undestand the technical solution at some point but from user point of view I think that there will be issues.
Personally I start from the user point of view how things will and should work and then I am focusing on the technical solution.

If I understood correctly VDs in the future will be Activities but without the extra space that someone might need for its windows.
That will break the workflow for the VDs users who are the majority of the linux desktop
(take note that I am not :). I am Activities heavy user that always uses one VD)).

If plasma devs agree on that roadmap then there are things that should be introduced in the future in order to make the VDs
users life a bit easier.

e.g. When a new Activity will be created shouldnt look exactly as the active one and be always in sync with it?
e.g. I am working on my current actitivity and I am writing a note on the desktop. I am creating a new Activity,
should that Activity look the same as the previous one and if I change the note in the first to look the same and in
the second? (at this example a current VDs user would answer should be in sync and always the same, a current
Activity user would answer it doesnt matter)

I fear that if this isnt orchestrated correctly with VDG a new debate for VDs and Activities
will relaunch just like the plasma 4 and 5 debates for the matter.

You can read my personal opinion for the matter at: https://psifidotos.blogspot.com/2012/03/activities-and-workareas-draft.html
I dont object in any alternative solution but the solution should be designed from
the user point of view first and then starting implementing.

abetts added a subscriber: abetts.Jul 3 2018, 2:44 PM

The dev team has been in talks with the VDG about this change. We are OK with the change. We understand that before we make any visual changes, we need to, at least, provide the same functionality that we have in X11 to what we want to have in Wayland. The VDG is currently working on devising the ways that this can work graphically.

hein added a comment.Jul 3 2018, 2:44 PM

Can we discuss this somewhere else, e.g. on plasma-devel or in one of the Plasma monday meetings? This is a review thread for a libtaskmanager code patch.

ngraham added a subscriber: ngraham.Jul 3 2018, 2:46 PM
In D13745#286544, @hein wrote:

Can we discuss this somewhere else, e.g. on plasma-devel or in one of the Plasma monday meetings? This is a review thread for a libtaskmanager code patch.

+1, let's do that elsewhere.

In D13745#286544, @hein wrote:

Can we discuss this somewhere else, e.g. on plasma-devel or in one of the Plasma monday meetings? This is a review thread for a libtaskmanager code patch.

of course, done...
I sent an e-mail to plasma mailing list with topic "Discussion for Virtual Desktops and Activities future"

zzag added a subscriber: zzag.Jul 3 2018, 7:12 PM
zzag added inline comments.
libtaskmanager/taskfilterproxymodel.h
73

Is it id or number?

libtaskmanager/virtualdesktopinfo.cpp
129–135

ids.reserve(KWindowSystem::numberOfDesktops());

321

Call reserve method. Also, why not range based for loop?

libtaskmanager/waylandtasksmodel.cpp
772

How about static_cast<size_t>(data.size())?

zzag added a comment.Jul 3 2018, 7:19 PM

Oh, please ignore my comments about reserve.

hein added inline comments.Jul 3 2018, 7:21 PM
libtaskmanager/taskfilterproxymodel.h
73

It's id.

libtaskmanager/waylandtasksmodel.cpp
772

No strong opinion :). This code is based on the one in the X11 model, which is in turn from the original libtaskmanager.

mart added a comment.Jul 4 2018, 4:00 PM
In D13745#286491, @hein wrote:

Michail: Please follow this review, as these API changes might impact Latte Dock, too. :)

  1. The idea is that Virtual Desktops will get some characteristics from Activities or vice versa?

this is what i think it will happen (in wayland and only in wayland)
the virtual desktops from kwin become the "source" for activities, so they are kept in sync: 1 activity = 1 desktop (this will probably be configurable to be enabled or disabled) that's possible because a window can be in any subset of desktops now, just like it could be on any subset of activities on X11.
So, if you want a window only shown on an activity, or a set of activities, you set it on those.
If you want a window to be there but change its contents depending from the activities, you set it on all desktops and use the kactivities api just as before

  1. The features provided from kactivitymanagerd will be dropped? (files linked to Activities, Activity icon, etc...) OR Virtual Desktops in Plasma will be identified through kactivitymanagerd in the future?

kactivitymanagerd will stay the same, just activities and virtual desktops would be the same set, any data, any stats, any linked files remain as now.

hein updated this revision to Diff 37201.Jul 5 2018, 7:03 PM
  • Adjust to Marco's API changes - builds again.
  • Add initial population loop to VirtualDesktopInfo on Marco's request.
hein updated this revision to Diff 37202.Jul 5 2018, 7:05 PM
  • Revert a bit flip that wasn't intended to be in there just yet.
hein updated this revision to Diff 37203.Jul 5 2018, 7:33 PM

Add API for creating/removing desktops.

hein updated this revision to Diff 37206.Jul 5 2018, 7:55 PM
  • Add AbstractTasksModel::requestNewVirtualDesktop and implement for Wayland and X11.
  • Remove VirtualDesktopInfo::canManageDesktops again as there's parity now.
hein updated this revision to Diff 37527.Jul 10 2018, 7:51 PM

Drop the population loop, it's as unnecessary as it should be.

hein updated this revision to Diff 38111.Jul 20 2018, 7:21 AM

Fixes for DND support code

anthonyfieroni added inline comments.
libtaskmanager/abstracttasksproxymodeliface.cpp
216

mapIfaceToSource and mapToSource returns non-const index i've wonder way it's get as const ref then performed a const_cast?

mart added a comment.Sep 28 2018, 4:14 PM

Api looks improved to me..
still some todo pieces, but looks good to me

libtaskmanager/taskgroupingproxymodel.cpp
725

what's the rationale of this dead code?

libtaskmanager/xwindowtasksmodel.cpp
633

{} initializer?

davidedmundson added inline comments.Oct 8 2018, 10:28 AM
libtaskmanager/taskgroupingproxymodel.cpp
725

should be mostly a copy/paste right? Anything holding this up?

1129

Am I reading this right?

We iterate backwards from N -> 1 because moving the desktop /might/ filter them.
Then because we might be at 1 child, we will have ungrouped so we change the root?

I would expect this to not work if a user is sorting by virtual desktops.

Would it work to generate a list of sourceIndexes from all children one loop, then go over those indexes requesting the desktops?

libtaskmanager/tasksmodel.cpp
1026–1027

leftover

hein added inline comments.Oct 9 2018, 3:08 AM
libtaskmanager/taskgroupingproxymodel.cpp
1129

Can you explain how sorting comes into this? (Sorting is done a level up in TasksModel.)

hein added inline comments.Oct 9 2018, 3:42 AM
libtaskmanager/xwindowtasksmodel.cpp
633

This style is used all over the codebase, better in a seperate patch.

hein added inline comments.Oct 9 2018, 3:45 AM
libtaskmanager/taskgroupingproxymodel.cpp
725

The sorting code will look at the first desktop in practice, which is why it should be the lowest desktop. I wasn't sure if I can rely on PlasmaWindow::plasmaVirtualDesktops() to return the desktops in position-sorting order. Something for @mart to answer.

hein added a comment.Oct 9 2018, 3:47 AM

I have prepared an update with the qDebug cleanup and the virtual desktops code in TaskGroupingProxyModel, but I don't want to risk my above questions being lost, so I'll hold of on updating before the answers are in.

mart added inline comments.Oct 10 2018, 9:33 AM
libtaskmanager/taskgroupingproxymodel.cpp
725

no, the desktops in plasmawindow are just in order of "last added last"
if is needed they always respect the order of their layout it can be done, tough would make the kwayland part more complicated

hein updated this revision to Diff 43317.Oct 10 2018, 3:24 PM
  • Make TaskGroupingProxyModel::requestVirtualDesktops and similar async-safe.
  • Make TasksModel::Private::lessThan handle unsorted virtual desktops lists.
davidedmundson accepted this revision.Oct 10 2018, 3:25 PM
This revision is now accepted and ready to land.Oct 10 2018, 3:25 PM
alexde added a subscriber: alexde.Oct 26 2018, 9:51 AM
This revision was automatically updated to reflect the committed changes.