[Wayland] Send stacking order event through plasma window management protocol
ClosedPublic

Authored by bport on Apr 21 2020, 2:55 PM.

Diff Detail

Repository
R108 KWin
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 25820
Build 25838: arc lint + arc unit
bport created this revision.Apr 21 2020, 2:55 PM
Restricted Application added a project: KWin. · View Herald TranscriptApr 21 2020, 2:55 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
bport requested review of this revision.Apr 21 2020, 2:55 PM
zzag added a comment.Apr 22 2020, 6:10 AM

I would rather prefer plasma_window to have a stacking order property so the plasma window management interface stays well encapsulated and to avoid using yet another type of window ids.

Feel free to push this change if others are okay with it.

abstract_client.h
875

Toplevel has already a property named internalId. I suggest to avoid using internalId. What about adding a m_windowManagementInterface accessor?

wayland_server.cpp
496–498

How's this possible?

bport planned changes to this revision.Apr 22 2020, 6:49 AM
bport added inline comments.
abstract_client.h
875

ok with that I wanted to avoid exposing too much, I will change that

wayland_server.cpp
496–498

I guess is now impossible (I have made this guard before executing this code inside a slot connected to workspaceInitialized

I would rather prefer plasma_window to have a stacking order property so the plasma window management interface stays well encapsulated and to avoid using yet another type of window ids.

The problem is that having that as the wire communication is that it's racey.
So assuming then we want this atomic list - I don't see a neat way to have the list, but not expose either windowInternalId or PlasmaWindow* from AbstractClient.

bport updated this revision to Diff 80890.Apr 22 2020, 2:07 PM

Expose plasma window management interface instead of internal id to avoid another window id exposed

On X we don't have this atomicity, so we can go to PlasmaWindow stacking order property without any regression.
However I think atomicity can prevent bug on future use.

zzag added inline comments.Apr 24 2020, 8:38 AM
wayland_server.cpp
496
  • No short variable names please.
  • Don't erase types if it makes reading code more difficult
for (Toplevel *toplevel : workspace()->stackingOrder()) {
    auto client = qobject_cast<AbstractClient *>(toplevel);
    // ...
}
bport updated this revision to Diff 81086.Apr 24 2020, 11:08 AM

take in consideration zzag review

davidedmundson accepted this revision.Apr 24 2020, 12:02 PM
This revision is now accepted and ready to land.Apr 24 2020, 12:02 PM
meven accepted this revision.Apr 24 2020, 12:52 PM
zzag requested changes to this revision.Apr 24 2020, 1:49 PM
zzag added inline comments.
wayland_server.cpp
496

stackingOrder() doesn't necessarily contains only clients. It can also contain Deleted toplevels. Please rename client to toplevel. We also put whitespace before and after :.

497

No snake_case please.

This revision now requires changes to proceed.Apr 24 2020, 1:49 PM
bport updated this revision to Diff 81303.Apr 27 2020, 7:12 AM

fix variable name

bport updated this revision to Diff 81306.Apr 27 2020, 7:31 AM

add space and rename variable

zzag accepted this revision.Apr 27 2020, 7:37 AM
This revision is now accepted and ready to land.Apr 27 2020, 7:37 AM
meven accepted this revision.Apr 27 2020, 9:55 AM
This revision was automatically updated to reflect the committed changes.