Implement input methods
Needs ReviewPublic

Authored by apol on Feb 12 2020, 1:39 AM.

Details

Reviewers
None
Group Reviewers
KWin
Summary

Allows to use external virtual keyboards and overall input infrastructure.

Depends on D27338

Diff Detail

Repository
R108 KWin
Branch
master
Lint
Lint ErrorsExcuse: xxx
SeverityLocationCodeMessage
Errorinput.h:310CppcheckunknownMacro
Errorinput.h:310CppcheckunknownMacro
Errorinput.h:310CppcheckunknownMacro
Errorinput.h:310CppcheckunknownMacro
Errorinput.h:310CppcheckunknownMacro
Errorinput.h:310CppcheckunknownMacro
Unit
No Unit Test Coverage
Build Status
Buildable 22383
Build 22401: arc lint + arc unit
apol created this revision.Feb 12 2020, 1:39 AM
Restricted Application added a project: KWin. · View Herald TranscriptFeb 12 2020, 1:39 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
apol requested review of this revision.Feb 12 2020, 1:39 AM
apol retitled this revision from a to Implement input methods.Feb 12 2020, 1:43 AM
apol edited the summary of this revision. (Show Details)
apol added a reviewer: KWin.
zzag added a subscriber: zzag.Feb 12 2020, 10:05 AM

FYI, in long term, we want to split XdgShellClient to better match abstractions in the xdg-shell spec and also to clean up code.

xdgshellclient.cpp
89–91

Is zwp_input_panel_surface_v1 yet another surface role, like xdg_toplevel or wl_subsurface?

apol added inline comments.Feb 12 2020, 1:37 PM
xdgshellclient.cpp
89–91

it's just an interface that provides us a surface and some information on how to place it on the screen.

zzag added inline comments.Feb 13 2020, 8:20 AM
xdgshellclient.cpp
89–91

I'm a little bit concerned about the case where one wants to get a zwp_input_panel_surface_v1 for a wl_surface which is also an xdg_toplevel or xdg_popup. In that case, the wl_surface will be represented by two XdgShellClient objects!

Perhaps we need to install an InputPanelSurfaceInterface rather than create a new XdgShellClient for it. For what it's worth, that's how we handle plasma surfaces (org_kde_plasma_surface).

void XdgShellClient::installInputPanelSurface(InputPanelSurfaceInterface *surface)
{
    m_inputPanelSurface = surface;

    // a bunch of connects
}
apol added inline comments.Feb 13 2020, 2:22 PM
xdgshellclient.cpp
89–91

I'm not sure that's a problem. I do know that at the moment the surfaces aren't created at all otherwise, be it from weston-keyboard or maliit (Qt).

@davidedmundson Do you have any ideas how to proceed?

xdgshellclient.cpp
89–91

oh, no.... it seems like zwp_input_panel_surface_v1 is yet another surface role, which means it can't be represented by XdgShellClient.

zzag added a comment.Mar 4 2020, 8:19 AM

So, judging by the source code of maliit, zwp_input_panel_surface_v1 is just yet another surface role, which means we can't use XdgShellClient for it, unfortunately. We need a separate AbstractClient subclass for input panel surfaces.

apol updated this revision to Diff 78512.Thu, Mar 26, 2:13 AM

Took into account the feedback I'd received, split XdgShellClient in two: the XdgShellClient and the WaylandClient which is a parent simpler class that can still deal with surfaces.

apol updated this revision to Diff 78513.Thu, Mar 26, 2:34 AM

readability

zzag added a comment.Thu, Mar 26, 8:40 AM

I came only halfway through the patch, but I don't think that it's a good idea to keep xdg-shell specific bits in WaylandClient.

abstract_client.h
869 ↗(On Diff #78513)

I don't see where it's implemented. Delete it? By the way, we have killWindow().

virtualkeyboard.cpp
135

Use frameGeometryChanged instead of geometryShapeChanged.

wayland_server.cpp
157–160

Leave it in createSurface(). You can't use xdg-foreign with non xdg_toplevel equivalent surfaces.

wayland_server.h
240

s/createInputPanelSurface/createInputPanelClient/

apol marked 4 inline comments as done.Thu, Mar 26, 3:06 PM

I came only halfway through the patch, but I don't think that it's a good idea to keep xdg-shell specific bits in WaylandClient.

I'm not sure which parts you mean. I tried to keep some bits so it's easier to re-use the class. If there's something that really rubs you the wrong way, I can look into moving.

That said, it seems to me like XdgShellClient could use its own splitting at some point and I don't want to overengineer the process at this stage.

apol updated this revision to Diff 78556.Thu, Mar 26, 3:06 PM

cleanups and vlad's comments