Changeset View
Standalone View
output.h
- This file was added.
1 | /******************************************************************** | ||||
---|---|---|---|---|---|
2 | KWin - the KDE window manager | ||||
3 | This file is part of the KDE project. | ||||
4 | | ||||
5 | Copyright 2018 Roman Gilg <subdiff@gmail.com> | ||||
6 | | ||||
7 | This program is free software; you can redistribute it and/or modify | ||||
8 | it under the terms of the GNU General Public License as published by | ||||
9 | the Free Software Foundation; either version 2 of the License, or | ||||
10 | (at your option) any later version. | ||||
11 | | ||||
12 | This program 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 | ||||
15 | GNU General Public License for more details. | ||||
16 | | ||||
17 | You should have received a copy of the GNU General Public License | ||||
18 | along with this program. If not, see <http://www.gnu.org/licenses/>. | ||||
19 | *********************************************************************/ | ||||
20 | #ifndef KWIN_OUTPUT_H | ||||
21 | #define KWIN_OUTPUT_H | ||||
22 | | ||||
23 | #include <utils.h> | ||||
24 | #include <kwin_export.h> | ||||
25 | | ||||
26 | #include <QObject> | ||||
27 | #include <QPointer> | ||||
28 | #include <QVector> | ||||
29 | #include <QPoint> | ||||
30 | #include <QSize> | ||||
31 | #include <QRect> | ||||
32 | | ||||
33 | namespace KWayland | ||||
34 | { | ||||
35 | namespace Server | ||||
36 | { | ||||
37 | class OutputInterface; | ||||
38 | class OutputDeviceInterface; | ||||
39 | class OutputChangeSet; | ||||
40 | class OutputManagementInterface; | ||||
41 | } | ||||
42 | } | ||||
43 | | ||||
44 | namespace KWin | ||||
45 | { | ||||
46 | | ||||
47 | /** | ||||
48 | * Generic output representation in a Wayland session | ||||
49 | **/ | ||||
50 | class KWIN_EXPORT Output : public QObject | ||||
51 | { | ||||
52 | Q_OBJECT | ||||
graesslin: I'm missing a ctor in this class. If you don't specify the default ctor is added which is not… | |||||
romangg: Hmm, this was similar to how DrmOutput was. But I will add one. | |||||
53 | public: | ||||
54 | virtual ~Output() {} | ||||
55 | | ||||
56 | QString name() const; | ||||
57 | bool isEnabled() const { | ||||
58 | return !m_waylandOutput.isNull(); | ||||
59 | } | ||||
60 | | ||||
61 | virtual QSize pixelSize() const = 0; | ||||
62 | qreal scale() const { | ||||
63 | return m_scale; | ||||
64 | } | ||||
65 | /* | ||||
66 | * The geometry of this output in global compositor co-ordinates (i.e scaled) | ||||
67 | */ | ||||
Don't refer to anything in code/comments as scaled. It could mean scaled from logical to device or could mean scaled from device to logical. davidedmundson: Don't refer to anything in code/comments as scaled.
It could mean scaled from logical to… | |||||
Ok, let me change this after merge. I forgot it now on the rebase and all the rebasing is nerve wrecking. romangg: Ok, let me change this after merge. I forgot it now on the rebase and all the rebasing is nerve… | |||||
68 | QRect geometry() const; | ||||
69 | QSize physicalSize() const; | ||||
70 | Qt::ScreenOrientation orientation() const { | ||||
71 | return m_orientation; | ||||
72 | } | ||||
73 | | ||||
74 | bool isInternal() const { | ||||
75 | return m_internal; | ||||
76 | } | ||||
77 | | ||||
78 | void setGlobalPos(const QPoint &pos); | ||||
79 | void setScale(qreal scale); | ||||
80 | | ||||
81 | /** | ||||
82 | * This sets the changes and tests them against the specific output | ||||
83 | */ | ||||
84 | void setChanges(KWayland::Server::OutputChangeSet *changeset); | ||||
85 | virtual bool commitChanges() { return false; } | ||||
86 | | ||||
87 | const QPointer<KWayland::Server::OutputInterface> getWaylandInterface() const { | ||||
88 | return m_waylandOutput; | ||||
89 | } | ||||
90 | | ||||
91 | protected: | ||||
92 | QPointer<KWayland::Server::OutputChangeSet> getChangeset() const { | ||||
please no protected variables (see e.g. https://softwareengineering.stackexchange.com/questions/162643/why-is-clean-code-suggesting-avoiding-protected-variables ) graesslin: please no protected variables (see e.g. https://softwareengineering.stackexchange. | |||||
93 | return m_changeset; | ||||
94 | } | ||||
95 | QPointer<KWayland::Server::OutputInterface> getWaylandOutput() const { | ||||
96 | return m_waylandOutput; | ||||
97 | } | ||||
98 | void setWaylandOutput(KWayland::Server::OutputInterface *set); | ||||
99 | QPointer<KWayland::Server::OutputDeviceInterface> getWaylandOutputDevice() const { | ||||
100 | return m_waylandOutputDevice; | ||||
101 | } | ||||
102 | void setWaylandOutputDevice(KWayland::Server::OutputDeviceInterface *set); | ||||
103 | QPoint globalPos() const { | ||||
104 | return m_globalPos; | ||||
105 | } | ||||
106 | | ||||
107 | QSize rawPhysicalSize() const { | ||||
108 | return m_physicalSize; | ||||
109 | } | ||||
110 | void setRawPhysicalSize(const QSize &set) { | ||||
111 | m_physicalSize = set; | ||||
112 | } | ||||
113 | | ||||
114 | void setOrientation(Qt::ScreenOrientation set) { | ||||
115 | m_orientation = set; | ||||
116 | } | ||||
117 | bool internal() const { | ||||
118 | return m_internal; | ||||
119 | } | ||||
120 | void setInternal(bool set) { | ||||
121 | m_internal = set; | ||||
122 | } | ||||
123 | | ||||
124 | private: | ||||
125 | QPointer<KWayland::Server::OutputChangeSet> m_changeset; | ||||
126 | QPointer<KWayland::Server::OutputInterface> m_waylandOutput; | ||||
127 | QPointer<KWayland::Server::OutputDeviceInterface> m_waylandOutputDevice; | ||||
128 | | ||||
129 | QPoint m_globalPos; | ||||
130 | qreal m_scale = 1; | ||||
131 | QSize m_physicalSize; | ||||
132 | Qt::ScreenOrientation m_orientation = Qt::PrimaryOrientation; | ||||
133 | bool m_internal = false; | ||||
134 | }; | ||||
135 | | ||||
I have problems understanding the ownership concept of these pointers. On the one hand they are QPointer, which would indicate they are owned by another class (otherwise QPointer just doesn't make any sense, the only advantage is that it tracks when a QObject gets destroyed), on the other hand the pointers are passed in as raw pointers and last but not least they are deleted in the dtor. If you want to have ownership of the pointers in this class (the delete in dtor) I suggest using a smart pointer which supports this concept (e.g. std::unique_ptr), if you want to have shared pointers, you could use std::shared_ptr or QSharedPointer. graesslin: I have problems understanding the ownership concept of these pointers. On the one hand they are… | |||||
The QPointers were also copied from DrmOutput. Were they used differently there? I wanted to copy as much semantic as possible without changes to minimize risk for regressions with this large patch. The objects are created in the Display class of KWayland, which shares ownership and for example KWayland::Server::Display::createOutput only returns a raw pointer. I assume that's why these were QPointers? romangg: The QPointers were also copied from DrmOutput. Were they used differently there? I wanted to… | |||||
136 | } | ||||
137 | | ||||
138 | #endif // KWIN_OUTPUT_H |
I'm missing a ctor in this class. If you don't specify the default ctor is added which is not typical for a QObject derived class. You want to have a ctor taking a QObject *parent = nullptr argument.