Differential D27338 Diff 78557 src/server/inputmethod_interface.cpp

Changeset View

Standalone View

View Options

src/server/inputmethod_interface.cpp

  • This file was added.
1​/*
2​ SPDX-FileCopyrightText: 2020 Aleix Pol Gonzalez <aleixpol@kde.org>
3​
4​ SPDX-License-Identifier: LGPL-2.1-only OR LGPL-3.0-only OR LicenseRef-KDE-Accepted-LGPL
5​*/
6​
7​#include "inputmethod_interface.h"
8​#include "resource_p.h"
9​#include "seat_interface.h"
10​#include "display.h"
11​#include "surface_interface.h"
12​#include "output_interface.h"
13​#include "surfacerole_p.h"
14​
15​#include <QHash>
16​#include "qwayland-server-input-method-unstable-v1.h"
17​#include "wayland-text-server-protocol.h"
18​
19​using namespace KWayland;
20​using namespace Server;
21​
22​static int s_version = 1;
23​
24​using namespace KWayland::Server;
25​
26​class Q_DECL_HIDDEN InputMethodContextInterface::Private : public QtWaylandServer::zwp_input_method_context_v1
zzagUnsubmitted
Done

Add Q_DECL_HIDDEN.

zzag: Add Q_DECL_HIDDEN.
apolAuthorUnsubmitted
Not Done

If it makes you happy. But we're building with opt-in export symbols, it shouldn't make much of a difference.

apol: If it makes you happy. But we're building with opt-in export symbols, it shouldn't make much of…
27​{
28​public:
29​ Private(InputMethodContextInterface *q)
30​ : zwp_input_method_context_v1()
31​ , q(q)
32​ {
33​ }
34​
35​ void zwp_input_method_context_v1_commit_string(Resource *, uint32_t serial, const QString &text) override
36​ {
37​ Q_EMIT q->commitString(serial, text);
38​ }
39​ void zwp_input_method_context_v1_preedit_string(Resource *, uint32_t serial, const QString &text, const QString &commit) override
40​ {
41​ Q_EMIT q->preeditString(serial, text, commit);
42​ }
43​
44​ void zwp_input_method_context_v1_preedit_styling(Resource *, uint32_t index, uint32_t length, uint32_t style) override
45​ {
46​ Q_EMIT q->preeditStyling(index, length, style);
47​ }
48​ void zwp_input_method_context_v1_preedit_cursor(Resource *, int32_t index) override
49​ {
50​ Q_EMIT q->preeditCursor(index);
51​ }
52​ void zwp_input_method_context_v1_delete_surrounding_text(Resource *, int32_t index, uint32_t length) override
53​ {
54​ Q_EMIT q->deleteSurroundingText(index, length);
55​ }
56​ void zwp_input_method_context_v1_cursor_position(Resource *, int32_t index, int32_t anchor) override
57​ {
58​ Q_EMIT q->cursorPosition(index, anchor);
59​ }
60​ void zwp_input_method_context_v1_modifiers_map(Resource *, wl_array *map) override
61​ {
62​ const QList<QByteArray> modifiersMap = QByteArray::fromRawData(static_cast<const char *>(map->data), map->size).split('\0');
63​
64​ mods.clear();
65​ for (const QByteArray &modifier : modifiersMap) {
66​ if (modifier == "Shift")
67​ mods << Qt::ShiftModifier;
68​ else if (modifier == "Alt")
69​ mods << Qt::AltModifier;
70​ else if (modifier == "Control")
71​ mods << Qt::ControlModifier;
72​ else if (modifier == "Mod1")
73​ mods << Qt::AltModifier;
74​ else if (modifier == "Mod4")
75​ mods << Qt::MetaModifier;
76​ else
77​ mods << Qt::NoModifier;
78​ }
79​ }
80​ void zwp_input_method_context_v1_keysym(Resource *, uint32_t serial, uint32_t time, uint32_t sym, uint32_t state, uint32_t modifiers) override
81​ {
82​ Q_EMIT q->keysym(serial, time, sym, state == WL_KEYBOARD_KEY_STATE_PRESSED, toQtModifiers(modifiers));
83​ }
84​ void zwp_input_method_context_v1_grab_keyboard(Resource *, uint32_t keyboard) override
85​ {
86​ Q_EMIT q->grabKeyboard(keyboard);
87​ }
88​ void zwp_input_method_context_v1_key(Resource *, uint32_t serial, uint32_t time, uint32_t key, uint32_t state) override
89​ {
90​ Q_EMIT q->key(serial, time, key, state == WL_KEYBOARD_KEY_STATE_PRESSED);
91​ }
92​ void zwp_input_method_context_v1_modifiers(Resource *, uint32_t serial, uint32_t mods_depressed, uint32_t mods_latched, uint32_t mods_locked, uint32_t group) override
93​ {
94​ Q_EMIT q->modifiers(serial, toQtModifiers(mods_depressed), toQtModifiers(mods_latched), toQtModifiers(mods_locked), group);
95​ }
96​ void zwp_input_method_context_v1_language(Resource *, uint32_t serial, const QString &language) override
97​ {
98​ Q_EMIT q->language(serial, language);
99​ }
100​ void zwp_input_method_context_v1_text_direction(Resource *, uint32_t serial, uint32_t direction) override
101​ {
102​ Qt::LayoutDirection qtDirection;
103​ switch (direction) {
104​ case WL_TEXT_INPUT_TEXT_DIRECTION_LTR:
105​ qtDirection = Qt::LeftToRight;
106​ break;
107​ case WL_TEXT_INPUT_TEXT_DIRECTION_RTL:
108​ qtDirection = Qt::RightToLeft;
109​ break;
110​ case WL_TEXT_INPUT_TEXT_DIRECTION_AUTO:
111​ qtDirection = Qt::LayoutDirectionAuto;
112​ break;
113​ }
114​ Q_EMIT q->textDirection(serial, qtDirection);
115​ }
116​
117​ Qt::KeyboardModifiers toQtModifiers(uint32_t modifiers)
118​ {
119​ Qt::KeyboardModifiers ret = Qt::NoModifier;
120​ for (int i = 0; modifiers >>= 1; ++i) {
121​ ret |= mods[i];
122​ }
123​ return ret;
124​ }
125​
126​ void zwp_input_method_context_v1_destroy_resource(Resource *resource) override
zzagUnsubmitted
Done

In the destructor request, we need to destroy the wl_resource with wl_resource_destroy(). When the resource is destroyed and it isn't inert, destroy q.

We probably need to destroy the wl_resource resource here.

wl_resource_destroy(resource->handle);
zzag: In the destructor request, we need to destroy the wl_resource with `wl_resource_destroy()`.
127​ {
128​ Q_UNUSED(resource)
129​ q->deleteLater();
zzagUnsubmitted
Not Done

Don't call wl_resource_destroy() in foobar_destroy_resource(), it's dangerous.

void zwp_input_method_context_v1_destroy(Resource *resource)
{
    wl_resource_destroy(resource->handle);
}

void zwp_input_method_context_v1_destroy_resource(Resource *resource)
{
    Q_UNUSED(resource)
    q->deleteLater();
}
zzag: Don't call wl_resource_destroy() in foobar_destroy_resource(), it's dangerous. ```lang=cpp…
130​ }
zzagUnsubmitted
Not Done

Destroy q in zwp_input_method_context_v1_destroy_resource(). In the destructor request, we usually want to destroy only the resource.

zzag: Destroy `q` in zwp_input_method_context_v1_destroy_resource(). In the destructor request, we…
131​
132​ void zwp_input_method_context_v1_destroy(Resource *resource) override
133​ {
134​ Q_EMIT q->destroy();
135​ wl_resource_destroy(resource->handle);
136​ }
137​
138​private:
139​ InputMethodContextInterface *const q;
140​ QVector<Qt::KeyboardModifiers> mods;
141​};
142​
143​InputMethodContextInterface::InputMethodContextInterface(InputMethodInterface *parent)
144​ : QObject(parent)
145​ , d(new InputMethodContextInterface::Private(this))
146​{
147​}
zzagUnsubmitted
Not Done

I also prefer not to put braces around for's and if's when the body contains only one statement. Code looks more compact; but we follow the Frameworks coding style and should put braces even if it's a single statement.

zzag: I also prefer not to put braces around for's and if's when the body contains only one statement.
148​
149​InputMethodContextInterface::~InputMethodContextInterface() = default;
150​
151​void InputMethodContextInterface::sendCommitState(uint32_t serial)
152​{
153​ for (auto r : d->resourceMap())
154​ d->send_commit_state(r->handle, serial);
zzagUnsubmitted
Not Done

I don't understand why we need resourceMap() here. zwp_input_method_context_v1 is not a global so I would expect to see something like

void InputMethodContextInterface::sendCommitState(uint32 serial)
{
    d->send_commit_state();
}
zzag: I don't understand why we need `resourceMap()` here. zwp_input_method_context_v1 is not a…
155​}
156​
157​void InputMethodContextInterface::sendContentType(uint32_t hint, uint32_t purpose)
158​{
159​ for (auto r : d->resourceMap())
160​ d->send_content_type(r->handle, hint, purpose);
161​}
162​
163​void InputMethodContextInterface::sendInvokeAction(uint32_t button, uint32_t index)
164​{
165​ for (auto r : d->resourceMap())
166​ d->send_invoke_action(r->handle, button, index);
167​}
168​
169​void InputMethodContextInterface::sendPreferredLanguage(const QString &language)
170​{
171​ for (auto r : d->resourceMap())
172​ d->send_preferred_language(r->handle, language);
173​}
174​
175​void InputMethodContextInterface::sendReset()
176​{
177​ for (auto r : d->resourceMap())
178​ d->send_reset(r->handle);
179​}
180​
zzagUnsubmitted
Done

Shouldn't InputPanelSurfaceInterface be also a subclass of SurfaceRole?

zzag: Shouldn't `InputPanelSurfaceInterface` be also a subclass of `SurfaceRole`?
apolAuthorUnsubmitted
Done

I don't know, just looked at it and it doesn't seem that useful?

apol: I don't know, just looked at it and it doesn't seem that useful?
zzagUnsubmitted
Done

zwp_input_panel_surface_v1 defines a surface role (based on weston code) so it should be a subclass of SurfaceRole. In long term, we want to do something like

SurfaceRole *role = SurfaceRole::get(surface);
if (role) {
    wl_resource_post_error(resource->handle, ZMY_SHELL_SURFACE_ERROR_ROLE,
                           "Cannot reassign surface role"); // DIE!
    return;
}
zzag: zwp_input_panel_surface_v1 defines a surface role (based on weston code) so it should be a…
181​void InputMethodContextInterface::sendSurroundingText(const QString &text, uint32_t cursor, uint32_t anchor)
182​{
zzagUnsubmitted
Not Done

We need to destroy the wl_resource for zwp_input_panel_surface_v1 when the associated surface is destroyed.

zzag: We need to destroy the wl_resource for zwp_input_panel_surface_v1 when the associated surface…
183​ for (auto r : d->resourceMap())
184​ d->send_surrounding_text(r->handle, text, cursor, anchor);
185​}
186​
187​class Q_DECL_HIDDEN InputPanelSurfaceInterface::Private : public QtWaylandServer::zwp_input_panel_surface_v1, public SurfaceRole
188​{
189​public:
190​ Private(InputPanelSurfaceInterface *q)
191​ : zwp_input_panel_surface_v1()
192​ , SurfaceRole()
193​ , q(q)
194​ {
195​ }
196​
197​ void zwp_input_panel_surface_v1_set_overlay_panel(Resource *) override
198​ {
199​ Q_EMIT q->overlayPanel();
200​ }
201​ void zwp_input_panel_surface_v1_set_toplevel(Resource *, struct ::wl_resource *output, uint32_t position) override
zzagUnsubmitted
Not Done

Naming nitpick: in FooPrivate classes, we avoid putting m_.

zzag: Naming nitpick: in `FooPrivate` classes, we avoid putting `m_`.
apolAuthorUnsubmitted
Not Done

I would rather not, otherwise when implementing private members they read like local variables.

apol: I would rather not, otherwise when implementing private members they read like local variables.
zzagUnsubmitted
Not Done

Frameworks' policies have no a single word about this so I guess it's okay(?) to put m_. I asked to drop m_ because Qt folks seem to prefer not to put m_ in private classes.

I would appreciate if you bring this topic up to the discussion in #kde-frameworks-devel since we're not consistent with naming in private classes.

zzag: Frameworks' policies have no a single word about this so I guess it's okay(?) to put `m_`. I…
202​ {
203​ Q_EMIT q->topLevel(OutputInterface::get(output), Position(position));
204​ }
205​
206​ void zwp_input_panel_surface_v1_destroy_resource(Resource * resource) override {
207​ wl_resource_destroy(resource->handle);
208​ if (resourceMap().isEmpty())
209​ q->deleteLater();
210​ }
zzagUnsubmitted
Not Done

Only delete q.

zzag: Only delete `q`.
211​
212​ void commit() override {}
213​
214​ InputPanelSurfaceInterface *const q;
215​ quint32 m_id = 0;
216​};
217​
218​InputPanelSurfaceInterface::InputPanelSurfaceInterface(QObject *parent)
219​ : QObject(parent)
220​ , d(new InputPanelSurfaceInterface::Private(this))
221​{
zzagUnsubmitted
Done

Naming nitpick: it would be nice to avoid names such as surfaceIface or surfaceInterface. I would like to highlight that surfaceIface is a bad name according to the Frameworks coding style.

Suggestion: rename surface to surfaceResource and then do auto surface = SurfaceInterface::get(surfaceResource);.

zzag: Naming nitpick: it would be nice to avoid names such as `surfaceIface` or `surfaceInterface`. I…
222​}
223​
224​InputPanelSurfaceInterface::~InputPanelSurfaceInterface() = default;
225​
zzagUnsubmitted
Done

No abbreviations.

zzag: No abbreviations.
226​class Q_DECL_HIDDEN InputPanelInterface::Private : public QObject, public QtWaylandServer::zwp_input_panel_v1
227​{
228​public:
229​ Private(InputPanelInterface *q, Display *d)
230​ : zwp_input_panel_v1(*d, s_version)
231​ , q(q)
232​ {
233​ }
234​
235​ void zwp_input_panel_v1_get_input_panel_surface(Resource *resource, uint32_t id, struct ::wl_resource *surfaceResource) override
236​ {
237​ auto interface = new InputPanelSurfaceInterface(nullptr);
238​ interface->d->init(resource->client(), id, resource->version());
239​ interface->d->m_id = id;
240​ interface->d->setSurface(SurfaceInterface::get(surfaceResource));
241​
242​ Q_EMIT q->inputPanelSurfaceAdded(interface);
243​ }
244​
245​ void zwp_input_panel_v1_destroy_resource(Resource *resource) override
246​ {
247​ wl_resource_destroy(resource->handle);
248​ if (resourceMap().isEmpty())
249​ q->deleteLater();
250​ }
251​
252​ InputPanelInterface *const q;
253​};
254​
255​InputPanelInterface::InputPanelInterface(Display *d, QObject *parent)
256​ : QObject(parent)
257​ , d(new InputPanelInterface::Private(this, d))
258​{
259​}
260​
261​InputPanelInterface::~InputPanelInterface() = default;
262​
263​quint32 InputPanelSurfaceInterface::id() const
264​{
265​ return d->m_id;
266​}
267​
268​SurfaceInterface *InputPanelSurfaceInterface::surface() const
269​{
270​ return d->surface();
271​}
272​
273​class Q_DECL_HIDDEN InputMethodInterface::Private : public QtWaylandServer::zwp_input_method_v1
274​{
275​public:
276​ Private(Display *d, InputMethodInterface *q)
277​ : zwp_input_method_v1(*d, s_version)
278​ , q(q)
279​ , m_display(d)
280​ {
281​ }
282​
283​ void zwp_input_method_v1_bind_resource(Resource *resource) override
284​ {
285​ if (!m_context)
286​ return;
287​
288​ auto x = m_context->d->add(resource->client(), resource->version());
289​
290​ if (m_enabled) {
291​ send_activate(resource->handle, x->handle);
292​ }
293​ }
294​
295​ InputMethodContextInterface *m_context = nullptr;
296​ InputMethodInterface *const q;
297​ Display *const m_display;
298​
299​ bool m_enabled = false;
300​};
301​
302​InputMethodInterface::InputMethodInterface(Display *d, QObject *parent)
303​ : QObject(parent)
304​ , d(new InputMethodInterface::Private(d, this))
305​{
306​}
307​
308​InputMethodInterface::~InputMethodInterface() = default;
309​
310​void InputMethodInterface::sendActivate()
311​{
312​ if (d->m_enabled)
313​ return;
314​
315​ Q_ASSERT(!d->m_context);
316​ d->m_context = new InputMethodContextInterface(this);
317​
318​ d->m_enabled = true;
319​ for (auto resource : d->resourceMap()) {
320​ auto connection = d->m_context->d->add(resource->client(), resource->version());
321​ d->send_activate(resource->handle, connection->handle);
322​ }
323​}
324​
325​void InputMethodInterface::sendDeactivate()
326​{
327​ if (!d->m_enabled)
328​ return;
329​
330​ d->m_enabled = false;
331​ for (auto resource : d->resourceMap()) {
332​ auto connection = d->m_context->d->resourceMap().value(resource->client());
333​ d->send_deactivate(connection->handle, resource->handle);
334​ }
335​ d->m_context = nullptr;
336​}
337​
338​InputMethodContextInterface *InputMethodInterface::context() const
339​{
340​ return d->m_context;
341​}