[xwl] Move Xwayland parts into separate class
ClosedPublic

Authored by romangg on Aug 23 2018, 11:02 AM.

Details

Summary

The Xwayland code path is moved from ApplicationWayland to a dedicated class
Xwayland in a new top-level directory xwl.

This is a direct preparation step for generic support of Xwayland Selections.

On a longer timescale this should also allow us to further separate Wayland
native functionality from Xwayland to allow us at one point to build KWin's
Wayland binary optionally without X dependencies. Another long term goal, that
becomes possible through this separation is to recover from Xwayland crashes.

Test Plan

Manually and auto tests

Diff Detail

Branch
0separateXwl
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2150
Build 2168: arc lint + arc unit
romangg created this revision.Aug 23 2018, 11:02 AM
Restricted Application added a project: KWin. · View Herald TranscriptAug 23 2018, 11:02 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Aug 23 2018, 11:02 AM
romangg retitled this revision from Separate Xwayland class to [xwl] Move Xwayland parts into separate class.Aug 23 2018, 11:14 AM
romangg edited the summary of this revision. (Show Details)
romangg edited the test plan for this revision. (Show Details)
romangg added a reviewer: KWin.
romangg edited the summary of this revision. (Show Details)Aug 23 2018, 11:20 AM
romangg updated this revision to Diff 40295.Aug 23 2018, 11:27 AM
  • Remove unrelated changes
romangg updated this revision to Diff 40296.Aug 23 2018, 11:27 AM

Use arc correctly.

romangg updated this revision to Diff 41383.Sep 11 2018, 10:12 AM

Rebase on master.

zzag added a subscriber: zzag.Sep 11 2018, 10:23 AM
zzag added inline comments.
xwl/xwayland.h
27

Is it used anywhere?

zzag added inline comments.Sep 11 2018, 11:27 AM
xwl/xwayland.h
27

OK, nvm, saw other patches.

davidedmundson added inline comments.
xwl/xwayland.cpp
236–237

This isn't really within the scope of an XWayland class as it's handling startup, stuff app should be doing.

More correct to emit something and have it handled with the same ApplicationWayland::continueStartupWithSceen

241

This isn't needed on wayland, ksplash is changed to ignore "wm" startup signals

romangg updated this revision to Diff 42006.Sep 20 2018, 8:42 PM

Rebase on master.

davidedmundson requested changes to this revision.Sep 28 2018, 11:31 AM

Marking as needs changes as I have a comment above not done., though I'm happy with the general direction of the patch

This revision now requires changes to proceed.Sep 28 2018, 11:31 AM
romangg added inline comments.Oct 22 2018, 1:37 PM
xwl/xwayland.cpp
236–237

True, the function is just copy and paste from ApplicationWayland::continueStartupWithX. I would like to leave it as is and do this later.

Best would be if we could omit all calls on m_app here.

241

Just copy and paste from old code. If it is unneeded even better. But I would like to put the removal in a separate patch then.

davidedmundson added inline comments.Oct 22 2018, 1:46 PM
xwl/xwayland.cpp
236–237

I would like to leave it as is and do this later.

It'll take like 5 minutes, and is within the scope of splitting out xwayland into a separate class.
I don't see a reason to do it later.

romangg added inline comments.Oct 22 2018, 3:43 PM
xwl/xwayland.cpp
236–237

As said, it is noted and shall be dealt with later.

I'm tired of arguing about such superficial stuff. If there is a functional defect with a patch I'm posting, I'm happy for every comment pointing it out and will change the diff of course, but if it's just about minor code cosmetics / structure let's try to avoid wasting time or worse, changing the diff on the fly for the sake of it and in the end risking a regression in a complex patch series.

There are real functional changes in the other patches of this series, which are of more importance to be studied thoroughly than the one we are arguing about right now.

romangg updated this revision to Diff 48517.Jan 2 2019, 11:05 AM

Rebase on master.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 19 2019, 11:10 AM
This revision was automatically updated to reflect the committed changes.