Differential D18569

Split out a dedicated InternalClient class
ClosedPublic
Actions

Authored by graesslin on Jan 27 2019, 7:52 PM.

Details

Reviewers

zzag

Group Reviewers

KWin

Commits

R108:9b922f88332f: Split out a dedicated InternalClient class

Summary

Most of the functionality which is special to internal clients is moved
from ShellClient to InternalClient. As KWin's qpa is still bound to the
Wayland protocol InternalClient inherits from ShellClient. Due to that
some aspects in ShellClient are "weird". ShellClient still detects
whether it's an internal client and uses the variable m_internal to
capture the state. This is required as we cannot use the isInternal
method. Most of m_internal usage is in init which is called from
constructor of ShellClient. Thus it's not possible to call into virtual
methods of InternalClient.

Also some of the code is duplicated and some methods are temporarily
marked as virtual.

The next step will be to remove ShmBuffer for internal windows which
should decouple the two implementations further with the long term goal
of having InternalClient inherit AbstractClient directly.

Test Plan

Run nested KWin, triggered outline (OpenGL case) and debug console (shm case).
InternalWindow unit test still passes.

Diff Detail

Repository

R108 KWin

Branch

internal-client

Lint

Lint OK

Unit

No Unit Test Coverage

Build Status

Buildable 7598
Build 7616: arc lint + arc unit

graesslin created this revision.Jan 27 2019, 7:52 PM

Restricted Application added a project: KWin. · View Herald TranscriptJan 27 2019, 7:52 PM

Restricted Application added a subscriber: kwin. · View Herald Transcript

graesslin requested review of this revision.Jan 27 2019, 7:52 PM

Harbormaster completed remote builds in B7598: Diff 50393.Jan 27 2019, 7:52 PM

Cool stuff!

Hmm, plasma surfaces are broken

(the panel should be at the bottom of the screen; notifications should appear in the bottom right corner of the screen)

(the panel should be at the bottom of the screen; notifications should appear in the bottom right corner of the screen)

I doubt that's from this patch, those windows aren't internal.
Do you maybe have my delayed init patch?

In D18569#403058, @davidedmundson wrote:

Do you maybe have my delayed init patch?

No, I don't.

I don't see how this patch could cause Plasma windows to have problems. All changed code is internal windows.

zzag added inline comments.Feb 10 2019, 5:41 PM

shell_client.cpp
1303	If I add `!m_internal` check back, plasma surfaces are no longer misplaced.

graesslin added inline comments.Feb 11 2019, 4:09 PM

shell_client.cpp
1303	ok, will update. It means the complete if needs to go. !m_internal is always true for Plasma shell so the second part never mattered.

Remove rect.isValid check

Harbormaster completed remote builds in B8159: Diff 51419.Feb 11 2019, 4:10 PM

zzag added inline comments.Feb 12 2019, 5:55 PM

shell_client.cpp
1303	Hmm, what about internal `Plasma::Dialog`s(e.g. an osd that shows up after switching between virtual desktops)? Won't this cause any issues?

graesslin added inline comments.Feb 16 2019, 1:25 PM

shell_client.cpp
1303	Yes and no. Currently this is broken. We have lots of bug reports about geometry being wrong for osd. This is in fact the main motivation for this change and the work on getting the QPA Wayland free. I consider that this won't work after the change, but it won't be broken more than currently and will allow to fixup properly afterwards.

Looks good to me.

shell_client.cpp
1303	Agreed, there are definitely some problems with OSDs, but I personally would prefer to keep things as they are. This change is about splitting internal entities in KWin, so users shouldn't see any difference before and after.

This revision is now accepted and ready to land.Feb 17 2019, 12:25 PM

By the way, ++ for "#pragma once". :-)

Closed by commit R108:9b922f88332f: Split out a dedicated InternalClient class (authored by graesslin). · Explain WhyFeb 21 2019, 6:53 PM

This revision was automatically updated to reflect the committed changes.

sbergeron added a subscriber: sbergeron.Jun 24 2019, 2:33 PM

Revision Contents
Changeset List

		Path
M		CMakeLists.txt (1 line)
M		autotests/integration/internal_window.cpp (2 lines)
A	M	internal_client.h (81 lines)
A	M	internal_client.cpp (321 lines)
M		shell_client.h (33 lines)
M		shell_client.cpp (160 lines)
M		wayland_server.cpp (8 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	50393	07e394f		Jan 27 2019, 7:52 PM	★	★
Diff 2	51419	07e394f	Remove rect.isValid check	Feb 11 2019, 4:10 PM	★	★
Diff 3	52232	66faa48	R108:9b922f88332fc470dcb50e9b55c7d69ab7da4d4c	Feb 21 2019, 5:51 PM	★	★