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
input
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24272
Build 24290: 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
90–92

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
90–92

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
90–92

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
90–92

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
90–92

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.Wed, Mar 4, 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

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

virtualkeyboard.cpp
134

Use frameGeometryChanged instead of geometryShapeChanged.

wayland_server.cpp
159–162

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

wayland_server.h
245

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