Differential D1989 Diff 5014 plugins/qml/kwinqml/kwinqml.cpp

Changeset View

Standalone View

View Options

plugins/qml/kwinqml/kwinqml.cpp

  • This file was added.
1​/********************************************************************
2​Copyright (C) 2016 Bhavisha Dhruve <bhavishadhruve@gmail.com>
3​
4​This program is free software; you can redistribute it and/or modify
5​it under the terms of the GNU General Public License as published by
6​the Free Software Foundation; either version 2 of the License, or
7​(at your option) any later version.
8​
9​This program is distributed in the hope that it will be useful,
10​but WITHOUT ANY WARRANTY; without even the implied warranty of
11​MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12​GNU General Public License for more details.
13​
14​You should have received a copy of the GNU General Public License
15​along with this program. If not, see <http://www.gnu.org/licenses/>.
16​*********************************************************************/
17​
18​#include "config-kwin.h"
19​#include "kwinqml.h"
20​#include <KWayland/Server/display.h>
21​#include <KWayland/Server/seat_interface.h>
22​#include <KWayland/Server/compositor_interface.h>
23​#include <KWayland/Server/shell_interface.h>
24​#include <QCoreApplication>
25​#include <QProcess>
26​#include <QDebug>
27​
28​// system
29​#include <unistd.h>
30​#include <sys/types.h>
31​#include <sys/socket.h>
32​
bshahUnsubmitted
Done

Coding style.

bshah: Coding style.
33​using namespace KWayland::Server;
34​
35​KWinQml::KWinQml(QQuickItem *parent) :QQuickItem(parent)
36​{
graesslinUnsubmitted
Done

the call to the parent class is missing

graesslin: the call to the parent class is missing
37​}
38​
39​KWinQml::~KWinQml()
40​{
41​ stop();
42​}
43​
44​void KWinQml::start()
45​{
davidedmundsonUnsubmitted
Not Done

this timer doesn't do anything, it's not started.

davidedmundson: this timer doesn't do anything, it's not started.
46​ Q_ASSERT(!m_display);
47​ m_display = new Display(this);
48​ m_display->start(Display::StartMode::ConnectClientsOnly);
graesslinUnsubmitted
Done

why check isNull and isEmpty?

graesslin: why check isNull and isEmpty?
49​ m_display->createShm();
50​ m_display->createCompositor()->create();
graesslinUnsubmitted
Done

actually we don't need that. The socketName is for the kwin process which we start. Our Display in this class is set to ConnectClientsOnly, thus it won't create a socket.

graesslin: actually we don't need that. The socketName is for the kwin process which we start. Our Display…
51​ m_seat = m_display->createSeat(m_display);
52​ m_seat->setHasTouch(true);
53​ m_seat->create();
54​ m_shell = m_display->createShell(m_display);
55​ m_shell->create();
56​ connect(m_shell, &ShellInterface::surfaceCreated, this,
57​ [this] (ShellSurfaceInterface *surface) {
graesslinUnsubmitted
Done

you also need to call create on m_shell.

graesslin: you also need to call create on m_shell.
58​ qDebug() << "Shell surface created";
59​ }
60​ );
61​ // output
62​ auto output = m_display->createOutput(m_display);
63​ const QSize size(height(), width());
64​ output->setGlobalPosition(QPoint(0, 0));
graesslinUnsubmitted
Done

don't use qDebug, please use categorized logging.

graesslin: don't use qDebug, please use categorized logging.
bshahUnsubmitted
Done

I'd say just remove this and debug statement below, they can get noisy, because it will be triggered at every frame repaint.

bshah: I'd say just remove this and debug statement below, they can get noisy, because it will be…
65​ output->setPhysicalSize(size / 3.8);
66​ output->addMode(size);
67​ output->create();
68​
69​ int sx[2];
graesslinUnsubmitted
Not Done

ideally you don't need to repaint everything, but could just update the actual changed areas (carried in the damaged signal)

graesslin: ideally you don't need to repaint everything, but could just update the actual changed areas…
bdhruveAuthorUnsubmitted
Not Done

I am not much knowledgeable about how to render only specific area in QtQuick, i tried to search for it, but couldn't find anything related to it.

bdhruve: I am not much knowledgeable about how to render only specific area in QtQuick, i tried to…
70​ if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sx) < 0) {
71​ return;
72​ }
73​ m_display->createClient(sx[0]);
74​ int socket = dup(sx[1]);
graesslinUnsubmitted
Done

There's one more env variable we should pass to the environment:
KWIN_COMPOSE=Q

In the long run we won't need that, but for the moment I think it makes our life easier. It forces the compositor to use QPainter instead of OpenGL, thus integrating the rendering will be way easier.

graesslin: There's one more env variable we should pass to the environment: KWIN_COMPOSE=Q In the long…
75​ if (socket == -1) {
76​ return;
bshahUnsubmitted
Done

Somehow seems you removed code for passing socket through WAYLAND_SOCKET environment variable and passing of --xwayland argument from this code. which was in previous revisions..

bshah: Somehow seems you removed code for passing socket through WAYLAND_SOCKET environment variable…
77​ }
78​ m_kwinWaylandProcess = new QProcess(this);
79​ m_kwinWaylandProcess->setProcessChannelMode(QProcess::ForwardedChannels);
80​ QProcessEnvironment environment = QProcessEnvironment::systemEnvironment();
graesslinUnsubmitted
Done

Please parse the arguments as:

QStringList arguments{QStringLiteral("--xwayland"), QStringLiteral("--socket"), m_socketName};

That way the list can be constructed in one go and not need to be appended multiple times.

graesslin: Please parse the arguments as: ``` QStringList arguments{QStringLiteral("--xwayland")…
81​ environment.insert(QStringLiteral("WAYLAND_SOCKET"), QString::fromUtf8(QByteArray::number(socket)));
graesslinUnsubmitted
Done

I think we need to pass a few more arguments:

  • --width
  • --height

with the values set to the size of this argument.

graesslin: I think we need to pass a few more arguments: - --width - --height with the values set…
82​ environment.insert(QStringLiteral("KWIN_COMPOSE"), QStringLiteral("Q"));
graesslinUnsubmitted
Done

we also need to pass at least --socket with a useful name. Otherwise it cannot be started in a Wayland session.

graesslin: we also need to pass at least --socket with a useful name. Otherwise it cannot be started in a…
graesslinUnsubmitted
Done

how is this instance being terminated again?

graesslin: how is this instance being terminated again?
83​ m_kwinWaylandProcess->setProcessEnvironment(environment);
84​ m_kwinWaylandProcess->setProcessChannelMode(QProcess::ForwardedChannels);
85​ QStringList arguments{QStringLiteral("--xwayland"),
86​ QStringLiteral("--socket"), m_socketName,
87​ QStringLiteral("--width"), QString::number(width()),
88​ QStringLiteral("--height"), QString::number(height())};
89​ m_kwinWaylandProcess->start(QStringLiteral(KWIN_WAYLAND_BINARY_PATH), arguments);
90​}
91​
92​void KWinQml::stop()
93​{
94​ m_kwinWaylandProcess->terminate();
95​}
graesslinUnsubmitted
Done

please watch coding style

graesslin: please watch coding style
graesslinUnsubmitted
Done

you cannot just assume that there's a buffer attached to the surface. Please perform a nullptr check

graesslin: you cannot just assume that there's a buffer attached to the surface. Please perform a nullptr…
bshahUnsubmitted
Done

You need to move this out of the if statement, otherwise only first frame will be rendered.

bshah: You need to move this out of the if statement, otherwise only first frame will be rendered.
graesslinUnsubmitted
Done

only do that on X11, if we are on Wayland that would be wrong. So do something like:

const int magicOffset = KWindowSystem::isPlatformX11() ? 8 : 0;
m_seat->keyPressed(event->nativeScanCode() - magicOffset);

And I'm glad it was the 8 issue, luckily I remembered that - having stumbled over this weirdness myself in the past.

graesslin: only do that on X11, if we are on Wayland that would be wrong. So do something like: const…
davidedmundsonUnsubmitted
Not Done

This texture leaks when this item is destroyed

davidedmundson: This texture leaks when this item is destroyed
davidedmundsonUnsubmitted
Not Done

you also want:

m_seat->setTimestamp(event->timestamp());

in all of these

and there's a wheelEvent(QWheelEvent*) that should be forwarded.

davidedmundson: you also want: m_seat->setTimestamp(event->timestamp()); in all of these and there's a…