Differential D1231 Diff 4100 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​/**
31​ * The structure server should fill to use this interface.
32​ * Lifecycle:
33​ * 1. GbmBuffer is filled and passed to RemoteAccessManager
34​ * (stored in manager's sent list)
35​ * 2. Client confirms that it wants this buffer, the RemoteBuffer
36​ * interface is then created and wrapped around GbmBuffer.
37​ * Manager purges its reference from sent list.
38​ * 3. Once client is done with buffer (or disconnected),
39​ * RemoteBuffer notifies manager and release signal is emitted.
romanggUnsubmitted
Not Done

rm whitespace (at the end)

romangg: rm whitespace (at the end)
40​ * It's the responsibility of process to delete this GbmBuffer
41​ * and release its' fd and gbm_bo afterwards.
42​ **/
43​struct GbmBuffer
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.
romanggUnsubmitted
Not Done

rm whitespace

romangg: rm whitespace
44​{
45​ // relevant for server
46​ qint32 fd = 0; //< also buffer_id
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​
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
48​ // Note that on client side received fd number will be different
49​ // and meaningful only for client process!
50​ // Thus we can use server-side fd as an implicit unique id
51​
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
52​ // relevant for client
53​ quint32 width = 0;
54​ quint32 height = 0;
55​ quint32 stride = 0;
56​ quint32 format = 0;
romanggUnsubmitted
Not Done

rm whitespace

romangg: rm whitespace
57​};
58​
59​namespace KWayland
60​{
61​namespace Server
62​{
63​
64​class Display;
65​
66​class KWAYLANDSERVER_EXPORT RemoteAccessManagerInterface : public Global
67​{
68​ Q_OBJECT
69​public:
70​ virtual ~RemoteAccessManagerInterface() = default;
71​
72​ /**
73​ * Store buffer in sent list and notify client that we have a buffer for it
74​ **/
75​ void sendBufferReady(const GbmBuffer *buf);
76​ /**
77​ * Check whether interface has been bound
78​ **/
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​ bool isBound() const;
80​
81​Q_SIGNALS:
82​ /**
83​ * Previously sent buffer has been released by client
84​ */
85​ void bufferReleased(const GbmBuffer *buf);
86​
87​private:
88​ explicit RemoteAccessManagerInterface(Display *display, QObject *parent = nullptr);
89​ friend class Display;
90​ class Private;
91​
92​};
93​
94​class KWAYLANDSERVER_EXPORT RemoteBufferInterface : public Resource
95​{
96​ Q_OBJECT
97​public:
98​ virtual ~RemoteBufferInterface() = default;
99​
100​ /**
101​ * Sends GBM fd to the client.
102​ * Note that server still has to close mirror fd from its side.
103​ **/
104​ void passFd();
105​
106​private:
107​ explicit RemoteBufferInterface(RemoteAccessManagerInterface *ram, wl_resource *pResource, const GbmBuffer *buf);
108​ friend class RemoteAccessManagerInterface;
109​
110​ class Private;
111​ Private *d_func() const;
112​};
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…
113​
114​
115​}
116​}
117​
118​#endif