Differential D27338 Diff 75505 src/server/inputmethod_interface.cpp

Changeset View

Standalone View

View Options

src/server/inputmethod_interface.cpp

  • This file was added.
1​/********************************************************************
2​Copyright 2020 Aleix Pol Gonzalez <aleixpol@kde.org>
3​
4​This library is free software; you can redistribute it and/or
5​modify it under the terms of the GNU Lesser General Public
6​License as published by the Free Software Foundation; either
7​version 2.1 of the License, or (at your option) version 3, or any
8​later version accepted by the membership of KDE e.V. (or its
9​successor approved by the membership of KDE e.V.), which shall
10​act as a proxy defined in Section 6 of version 3 of the license.
11​
12​This library is distributed in the hope that it will be useful,
13​but WITHOUT ANY WARRANTY; without even the implied warranty of
14​MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15​Lesser General Public License for more details.
16​
17​You should have received a copy of the GNU Lesser General Public
18​License along with this library. If not, see <http://www.gnu.org/licenses/>.
19​*********************************************************************/
20​
21​#include "inputmethod_interface.h"
22​#include "resource_p.h"
23​#include "seat_interface.h"
24​#include "display.h"
25​#include "surface_interface.h"
26​#include "output_interface.h"
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​#include <QHash>
29​#include "qwayland-server-input-method-unstable-v1.h"
30​#include "wayland-text-server-protocol.h"
31​
32​using namespace KWayland;
33​using namespace Server;
34​
35​static int s_version = 1;
36​
37​using namespace KWayland::Server;
38​
39​class InputMethodContextInterface::Private : public QtWaylandServer::zwp_input_method_context_v1
40​{
41​public:
42​ Private(InputMethodInterface *method, InputMethodContextInterface *q)
43​ : zwp_input_method_context_v1()
44​ , q(q)
45​ {
46​ }
47​
48​ void zwp_input_method_context_v1_commit_string(Resource *, uint32_t serial, const QString &text) override
49​ {
50​ Q_EMIT q->commitString(serial, text);
51​ }
52​ void zwp_input_method_context_v1_preedit_string(Resource *, uint32_t serial, const QString &text, const QString &commit) override
53​ {
54​ Q_EMIT q->preeditString(serial, text, commit);
55​ }
56​
57​ void zwp_input_method_context_v1_preedit_styling(Resource *, uint32_t index, uint32_t length, uint32_t style) override
58​ {
59​ Q_EMIT q->preeditStyling(index, length, style);
60​ }
61​ void zwp_input_method_context_v1_preedit_cursor(Resource *, int32_t index) override
62​ {
63​ Q_EMIT q->preeditCursor(index);
64​ }
65​ void zwp_input_method_context_v1_delete_surrounding_text(Resource *, int32_t index, uint32_t length) override
66​ {
67​ Q_EMIT q->deleteSurroundingText(index, length);
68​ }
69​ void zwp_input_method_context_v1_cursor_position(Resource *, int32_t index, int32_t anchor) override
70​ {
71​ Q_EMIT q->cursorPosition(index, anchor);
72​ }
73​ void zwp_input_method_context_v1_modifiers_map(Resource *, wl_array *map) override
74​ {
75​ const QList<QByteArray> modifiersMap = QByteArray::fromRawData(static_cast<const char *>(map->data), map->size).split('\0');
76​
77​ mods.clear();
78​ for (const QByteArray &modifier : modifiersMap) {
79​ if (modifier == "Shift")
80​ mods << Qt::ShiftModifier;
81​ else if (modifier == "Alt")
82​ mods << Qt::AltModifier;
83​ else if (modifier == "Control")
84​ mods << Qt::ControlModifier;
85​ else if (modifier == "Mod1")
86​ mods << Qt::AltModifier;
87​ else if (modifier == "Mod4")
88​ mods << Qt::MetaModifier;
89​ else
90​ mods << Qt::NoModifier;
91​ }
92​ }
93​ void zwp_input_method_context_v1_keysym(Resource *, uint32_t serial, uint32_t time, uint32_t sym, uint32_t state, uint32_t modifiers) override
94​ {
95​ Q_EMIT q->keysym(serial, time, sym, state == WL_KEYBOARD_KEY_STATE_PRESSED, toQtModifiers(modifiers));
96​ }
97​ void zwp_input_method_context_v1_grab_keyboard(Resource *, uint32_t keyboard) override
98​ {
99​ Q_EMIT q->grabKeyboard(keyboard);
100​ }
101​ void zwp_input_method_context_v1_key(Resource *, uint32_t serial, uint32_t time, uint32_t key, uint32_t state) override
102​ {
103​ Q_EMIT q->key(serial, time, key, state == WL_KEYBOARD_KEY_STATE_PRESSED);
104​ }
105​ 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
106​ {
107​ Q_EMIT q->modifiers(serial, toQtModifiers(mods_depressed), toQtModifiers(mods_latched), toQtModifiers(mods_locked), group);
108​ }
109​ void zwp_input_method_context_v1_language(Resource *, uint32_t serial, const QString &language) override
110​ {
111​ Q_EMIT q->language(serial, language);
112​ }
113​ void zwp_input_method_context_v1_text_direction(Resource *, uint32_t serial, uint32_t direction) override
114​ {
115​ Qt::LayoutDirection qtDirection;
116​ switch (direction) {
117​ case WL_TEXT_INPUT_TEXT_DIRECTION_LTR:
118​ qtDirection = Qt::LeftToRight;
119​ break;
120​ case WL_TEXT_INPUT_TEXT_DIRECTION_RTL:
121​ qtDirection = Qt::RightToLeft;
122​ break;
123​ case WL_TEXT_INPUT_TEXT_DIRECTION_AUTO:
124​ qtDirection = Qt::LayoutDirectionAuto;
125​ break;
126​ }
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​ Q_EMIT q->textDirection(serial, qtDirection);
128​ }
129​
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​ Qt::KeyboardModifiers toQtModifiers(uint32_t modifiers)
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​ Qt::KeyboardModifiers ret = Qt::NoModifier;
133​ for (int i = 0; modifiers >>= 1; ++i) {
134​ ret |= mods[i];
135​ }
136​ return ret;
137​ }
138​
139​ void zwp_input_method_context_v1_destroy(QtWaylandServer::zwp_input_method_context_v1::Resource *resource) override
140​ {
141​ q->deleteLater();
142​ }
143​
144​private:
145​ InputMethodContextInterface *const q;
146​ QVector<Qt::KeyboardModifiers> mods;
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(InputMethodInterface *parent)
150​ : QObject(parent)
151​ , d(new InputMethodContextInterface::Private(parent, this))
152​{
153​}
154​
155​InputMethodContextInterface::~InputMethodContextInterface() = default;
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…
156​
157​void InputMethodContextInterface::sendCommitState(uint32_t serial)
158​{
159​ for (auto r : d->resourceMap())
160​ d->send_commit_state(r->handle, serial);
161​}
162​
163​void InputMethodContextInterface::sendContentType(uint32_t hint, uint32_t purpose)
164​{
165​ for (auto r : d->resourceMap())
166​ d->send_content_type(r->handle, hint, purpose);
167​}
168​
169​void InputMethodContextInterface::sendInvokeAction(uint32_t button, uint32_t index)
170​{
171​ for (auto r : d->resourceMap())
172​ d->send_invoke_action(r->handle, button, index);
173​}
174​
175​void InputMethodContextInterface::sendPreferredLanguage(const QString &language)
176​{
177​ for (auto r : d->resourceMap())
178​ d->send_preferred_language(r->handle, language);
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::sendReset()
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_reset(r->handle);
185​}
186​
187​void InputMethodContextInterface::sendSurroundingText(const QString &text, uint32_t cursor, uint32_t anchor)
188​{
189​ for (auto r : d->resourceMap())
190​ d->send_surrounding_text(r->handle, text, cursor, anchor);
191​}
192​
193​class InputPanelSurfaceInterface::Private : public QtWaylandServer::zwp_input_panel_surface_v1
194​{
195​public:
196​ Private(InputPanelSurfaceInterface *q)
197​ : zwp_input_panel_surface_v1()
198​ , q(q)
199​ {
200​ }
201​
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​ void zwp_input_panel_surface_v1_set_overlay_panel(Resource *) override
203​ {
204​ Q_EMIT q->overlayPanel();
205​ }
206​ void zwp_input_panel_surface_v1_set_toplevel(Resource *, struct ::wl_resource *output, uint32_t position) override
207​ {
208​ OutputInterface *outputIface = OutputInterface::get(output);
209​ Q_EMIT q->topLevel(outputIface, Position(position));
210​ }
211​
zzagUnsubmitted
Not Done

Only delete q.

zzag: Only delete `q`.
212​ InputPanelSurfaceInterface *const q;
213​ QPointer<SurfaceInterface> m_surface;
214​ bool m_overlay = false;
215​};
216​
217​InputPanelSurfaceInterface::InputPanelSurfaceInterface(QObject *parent)
218​ : QObject(parent)
219​ , d(new InputPanelSurfaceInterface::Private(this))
220​{
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​InputPanelSurfaceInterface::~InputPanelSurfaceInterface() = default;
224​
225​class InputPanelInterface::Private : public QObject, public QtWaylandServer::zwp_input_panel_v1
zzagUnsubmitted
Done

No abbreviations.

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