Differential D1231 Diff 3620 src/server/remote_access_interface.h

Changeset View

Standalone View

View Options

src/server/remote_access_interface.h

  • This file was added.
1​/****************************************************************************
2​Copyright 2016 Oleg Chernovskiy <kanedias@xaker.ru>
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​#ifndef KWAYLAND_SERVER_REMOTE_ACCESS_H
21​#define KWAYLAND_SERVER_REMOTE_ACCESS_H
22​
23​#include "global.h"
24​#include "resource.h"
25​
26​#include <KWayland/Server/kwaylandserver_export.h>
27​
28​#include <QHash>
29​
30​class gbm_surface;
31​class gbm_bo;
32​
33​/**
34​ * The structure server should fill to use this interface.
35​ * Lifecycle:
36​ * 1. GbmBuffer is filled and passed to RemoteAccessManager
37​ * (stored in manager's sent list)
38​ * 2. Client confirms that it wants this buffer, the RemoteBuffer
39​ * interface is then created and wrapped around GbmBuffer.
romanggUnsubmitted
Not Done

rm whitespace (at the end)

romangg: rm whitespace (at the end)
40​ * Manager purges its reference from sent list.
41​ * 3. Once client is done with buffer (or disconnected),
42​ * RemoteBuffer notifies manager and release signal is emitted.
43​ * It's the responsibility of process to delete this GbmBuffer
romanggUnsubmitted
Not Done

rm whitespace

romangg: rm whitespace
44​ * and release its' fd and gbm_bo afterwards.
graesslinUnsubmitted
Done

for ABI compatibility we cannot have structs defined in the header. See https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B#The_Do.27s_and_Don.27ts for explanation. Especially the last point of the Donts is relevant as it means we are never able to change this again.

Thus I think the proper way has to be:

class Buffer
{
public:
void setFd(int qint32);
void setSize(const QSize &size);
void setStride(qint32 stride);
enum class Format {
Format1,
Format2
};
void setFormat(Format format);

private:
class Private;
QScopedPointer<Private> d;
};

and then in the Implementation have the Private class which holds the data members.

graesslin: for ABI compatibility we cannot have structs defined in the header. See https://community.kde.
KanediasAuthorUnsubmitted
Not Done

Thanks for clarification.
Reimplemented.

Kanedias: Thanks for clarification. Reimplemented.
45​ **/
46​struct GbmBuffer
graesslinUnsubmitted
Done

For ABI compatibility it's better to only have d-ptr-ized classes/structs in the public header.

graesslin: For ABI compatibility it's better to only have d-ptr-ized classes/structs in the public header.
KanediasAuthorUnsubmitted
Not Done

Implemented GbmBuffer with d-pointer

Kanedias: Implemented GbmBuffer with d-pointer
graesslinUnsubmitted
Done

Thinking out loud:

What if in future we pass non GbmBuffers? Should we then still call it GbmBuffer or do we need a new class? Maybe we should make it a nested class to RemoteAccessManagerInterface and then just call it Buffer? Or keep it outside and call it more generic RemoteBuffer?

graesslin: Thinking out loud: What if in future we pass non GbmBuffers? Should we then still call it…
KanediasAuthorUnsubmitted
Not Done

Yes, it will no longer be GBM. Seeing so much EGLStreams and Vulkan struggles around it would be good to rename it into something more generic... noted!

Kanedias: Yes, it will no longer be GBM. Seeing so much EGLStreams and Vulkan struggles around it would…
47​{
48​ // relevant for server
49​ gbm_surface *surface = nullptr;
50​ gbm_bo *bo = nullptr;
graesslinUnsubmitted
Done

why do we need gbm_surface and gbm_bo? This looks like adding not needed details to the interface

graesslin: why do we need gbm_surface and gbm_bo? This looks like adding not needed details to the…
KanediasAuthorUnsubmitted
Not Done

Removed, reimplemented

Kanedias: Removed, reimplemented
51​ qint32 fd = 0; //< also buffer_id
52​
53​ // Note that on client side received fd number will be different
54​ // and meaningful only for client process!
55​ // Thus we can use server-side fd as an implicit unique id
56​
romanggUnsubmitted
Not Done

rm whitespace

romangg: rm whitespace
57​ // relevant for client
58​ quint32 width = 0;
59​ quint32 height = 0;
60​ quint32 stride = 0;
61​ quint32 format = 0;
62​};
63​
64​namespace KWayland
65​{
66​namespace Server
67​{
68​
69​class Display;
70​
71​class KWAYLANDSERVER_EXPORT RemoteAccessManagerInterface : public Global
72​{
73​ Q_OBJECT
74​public:
75​ virtual ~RemoteAccessManagerInterface() = default;
76​
77​ /**
78​ * Store buffer in sent list and notify client that we have a buffer for it
graesslinUnsubmitted
Done

who's the owner of the GbmBuffer? Who will delete it?

graesslin: who's the owner of the GbmBuffer? Who will delete it?
KanediasAuthorUnsubmitted
Not Done

The owner is the process that originally passed it. So KWin here. In my non-submitted PoC from KWin side it closes its fd and deletes it after bufferReleased call

Kanedias: The owner is the process that originally passed it. So KWin here. In my non-submitted PoC from…
79​ **/
80​ void sendBufferReady(const GbmBuffer *buf);
81​ /**
82​ * Check whether interface has been bound
83​ **/
84​ bool isBound() const;
85​
86​Q_SIGNALS:
87​ /**
88​ * Previously sent buffer has been released by client
89​ */
90​ void bufferReleased(const GbmBuffer *buf);
91​
92​private:
93​ explicit RemoteAccessManagerInterface(Display *display, QObject *parent = nullptr);
94​ friend class Display;
95​ class Private;
96​
97​};
98​
99​class KWAYLANDSERVER_EXPORT RemoteBufferInterface : public Resource
100​{
101​ Q_OBJECT
102​public:
103​ virtual ~RemoteBufferInterface() = default;
104​
105​ /**
106​ * Sends GBM fd to the client.
107​ * Note that server still has to close mirror fd from its side.
108​ **/
109​ void passFd();
110​
111​private:
112​ explicit RemoteBufferInterface(RemoteAccessManagerInterface *ram, wl_resource *pResource, const GbmBuffer *buf);
113​ friend class RemoteAccessManagerInterface;
graesslinUnsubmitted
Not Done

I don't see this class exposed in any way in the API. So a user of the library cannot have access to it.

Thus I suggest to move it into a private header and remove the KWAYLANDSERVER_EXPORT. It's a private class to the library then, which makes life easier.

graesslin: I don't see this class exposed in any way in the API. So a user of the library cannot have…
114​
115​ class Private;
116​ Private *d_func() const;
117​};
118​
119​
120​}
121​}
122​
123​#endif