From 2b7fa206e8efc6589b34d651c456333732c239dc Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Fri, 15 Apr 2022 20:06:45 +0300 Subject: [PATCH] Refactor Output information initialization Instead of passing all possible field values to the initialize() function, pass all relevant data in a struct. With designated initializers, it's more readable and makes code more comprehensible. The general goal is to split Output's data in two categories - general information about the output (e.g. edid) and mutable state (position, mode, etc). --- src/backends/drm/drm_backend.cpp | 4 +- src/backends/drm/drm_gpu.cpp | 5 +- src/backends/drm/drm_gpu.h | 9 +-- src/backends/drm/drm_output.cpp | 50 ++++++++------- src/backends/drm/drm_output.h | 2 - src/backends/drm/drm_virtual_output.cpp | 22 +++---- src/backends/drm/drm_virtual_output.h | 9 ++- src/backends/virtual/virtual_output.cpp | 10 +-- src/backends/wayland/wayland_output.cpp | 9 ++- src/backends/x11/standalone/x11_output.cpp | 8 +-- src/backends/x11/standalone/x11_output.h | 3 +- src/backends/x11/standalone/x11_platform.cpp | 8 ++- .../x11/standalone/x11placeholderoutput.cpp | 12 ++-- .../x11/windowed/x11windowed_output.cpp | 8 +-- src/output.cpp | 62 +++++-------------- src/output.h | 41 +++++------- 16 files changed, 102 insertions(+), 160 deletions(-) diff --git a/src/backends/drm/drm_backend.cpp b/src/backends/drm/drm_backend.cpp index e84cb6a78d..d0cecbc2b9 100644 --- a/src/backends/drm/drm_backend.cpp +++ b/src/backends/drm/drm_backend.cpp @@ -524,7 +524,7 @@ void DrmBackend::enableOutput(DrmAbstractOutput *output, bool enable) } if (m_enabledOutputs.count() == 1 && !kwinApp()->isTerminating()) { qCDebug(KWIN_DRM) << "adding placeholder output"; - m_placeHolderOutput = primaryGpu()->createVirtualOutput({}, m_enabledOutputs.constFirst()->pixelSize(), 1, DrmGpu::Placeholder); + m_placeHolderOutput = primaryGpu()->createVirtualOutput({}, m_enabledOutputs.constFirst()->pixelSize(), 1, DrmVirtualOutput::Type::Placeholder); // placeholder doesn't actually need to render anything m_placeHolderOutput->renderLoop()->inhibit(); m_placeholderFilter.reset(new PlaceholderInputEventFilter()); @@ -586,7 +586,7 @@ QString DrmBackend::supportInformation() const Output *DrmBackend::createVirtualOutput(const QString &name, const QSize &size, double scale) { - auto output = primaryGpu()->createVirtualOutput(name, size * scale, scale, DrmGpu::Full); + auto output = primaryGpu()->createVirtualOutput(name, size * scale, scale, DrmVirtualOutput::Type::Virtual); readOutputsConfiguration(m_outputs); Q_EMIT screensQueried(); return output; diff --git a/src/backends/drm/drm_gpu.cpp b/src/backends/drm/drm_gpu.cpp index e599be1f52..52182b50a6 100644 --- a/src/backends/drm/drm_gpu.cpp +++ b/src/backends/drm/drm_gpu.cpp @@ -582,11 +582,10 @@ const QVector DrmGpu::pipelines() const return m_pipelines; } -DrmVirtualOutput *DrmGpu::createVirtualOutput(const QString &name, const QSize &size, double scale, VirtualOutputMode mode) +DrmVirtualOutput *DrmGpu::createVirtualOutput(const QString &name, const QSize &size, double scale, DrmVirtualOutput::Type type) { - auto output = new DrmVirtualOutput(name, this, size); + auto output = new DrmVirtualOutput(name, this, size, type); output->setScale(scale); - output->setPlaceholder(mode == Placeholder); m_outputs << output; Q_EMIT outputEnabled(output); Q_EMIT outputAdded(output); diff --git a/src/backends/drm/drm_gpu.h b/src/backends/drm/drm_gpu.h index 99bd556c31..d30c137ab0 100644 --- a/src/backends/drm/drm_gpu.h +++ b/src/backends/drm/drm_gpu.h @@ -10,6 +10,8 @@ #ifndef DRM_GPU_H #define DRM_GPU_H +#include "drm_virtual_output.h" + #include #include #include @@ -39,7 +41,6 @@ class DrmBackend; class EglGbmBackend; class DrmPipeline; class DrmAbstractOutput; -class DrmVirtualOutput; class DrmLeaseOutput; class DrmRenderBackend; @@ -74,11 +75,7 @@ public: bool updateOutputs(); - enum VirtualOutputMode { - Placeholder, - Full, - }; - DrmVirtualOutput *createVirtualOutput(const QString &name, const QSize &size, double scale, VirtualOutputMode mode); + DrmVirtualOutput *createVirtualOutput(const QString &name, const QSize &size, double scale, DrmVirtualOutput::Type type); void removeVirtualOutput(DrmVirtualOutput *output); bool testPendingConfiguration(); diff --git a/src/backends/drm/drm_output.cpp b/src/backends/drm/drm_output.cpp index 5af27c1353..189664f639 100644 --- a/src/backends/drm/drm_output.cpp +++ b/src/backends/drm/drm_output.cpp @@ -48,22 +48,42 @@ DrmOutput::DrmOutput(DrmPipeline *pipeline) m_pipeline->setOutput(this); const auto conn = m_pipeline->connector(); m_renderLoop->setRefreshRate(m_pipeline->pending.mode->refreshRate()); - setSubPixelInternal(conn->subpixel()); - setInternal(conn->isInternal()); - setCapabilityInternal(DrmOutput::Capability::Dpms); + + Capabilities capabilities = Capability::Dpms; if (conn->hasOverscan()) { - setCapabilityInternal(Capability::Overscan); + capabilities |= Capability::Overscan; setOverscanInternal(conn->overscan()); } if (conn->vrrCapable()) { - setCapabilityInternal(Capability::Vrr); + capabilities |= Capability::Vrr; setVrrPolicy(RenderLoop::VrrPolicy::Automatic); } if (conn->hasRgbRange()) { - setCapabilityInternal(Capability::RgbRange); + capabilities |= Capability::RgbRange; setRgbRangeInternal(conn->rgbRange()); } - initOutputDevice(); + + const Edid *edid = conn->edid(); + + setInformation(Information{ + .name = conn->connectorName(), + .manufacturer = edid->manufacturerString(), + .model = conn->modelName(), + .serialNumber = edid->serialNumber(), + .eisaId = edid->eisaId(), + .physicalSize = conn->physicalSize(), + .edid = edid->raw(), + .subPixel = conn->subpixel(), + .capabilities = capabilities, + .internal = conn->isInternal(), + }); + + const QList> modes = getModes(); + QSharedPointer currentMode = m_pipeline->pending.mode; + if (!currentMode) { + currentMode = modes.constFirst(); + } + setModesInternal(modes, currentMode); m_turnOffTimer.setSingleShot(true); m_turnOffTimer.setInterval(dimAnimationTime()); @@ -177,22 +197,6 @@ QList> DrmOutput::getModes() const return ret; } -void DrmOutput::initOutputDevice() -{ - const auto conn = m_pipeline->connector(); - setName(conn->connectorName()); - initialize(conn->modelName(), conn->edid()->manufacturerString(), - conn->edid()->eisaId(), conn->edid()->serialNumber(), - conn->physicalSize(), conn->edid()->raw()); - - const QList> modes = getModes(); - QSharedPointer currentMode = m_pipeline->pending.mode; - if (!currentMode) { - currentMode = modes.constFirst(); - } - setModesInternal(modes, currentMode); -} - void DrmOutput::updateEnablement(bool enable) { m_gpu->platform()->enableOutput(this, enable); diff --git a/src/backends/drm/drm_output.h b/src/backends/drm/drm_output.h index 43891e6c07..031ce05a9c 100644 --- a/src/backends/drm/drm_output.h +++ b/src/backends/drm/drm_output.h @@ -60,8 +60,6 @@ public: void setColorTransformation(const QSharedPointer &transformation) override; private: - void initOutputDevice(); - void updateEnablement(bool enable) override; bool setDrmDpmsMode(DpmsMode mode); void setDpmsMode(DpmsMode mode) override; diff --git a/src/backends/drm/drm_virtual_output.cpp b/src/backends/drm/drm_virtual_output.cpp index b7356efc13..c04f43ce13 100644 --- a/src/backends/drm/drm_virtual_output.cpp +++ b/src/backends/drm/drm_virtual_output.cpp @@ -20,30 +20,22 @@ namespace KWin { -static int s_serial = 0; -DrmVirtualOutput::DrmVirtualOutput(DrmGpu *gpu, const QSize &size) - : DrmVirtualOutput(QString::number(s_serial++), gpu, size) -{ -} - -DrmVirtualOutput::DrmVirtualOutput(const QString &name, DrmGpu *gpu, const QSize &size) +DrmVirtualOutput::DrmVirtualOutput(const QString &name, DrmGpu *gpu, const QSize &size, Type type) : DrmAbstractOutput(gpu) , m_vsyncMonitor(SoftwareVsyncMonitor::create(this)) { connect(m_vsyncMonitor, &VsyncMonitor::vblankOccurred, this, &DrmVirtualOutput::vblank); - setName("Virtual-" + name); - initialize(QLatin1String("model_") + name, - QLatin1String("manufacturer_") + name, - QLatin1String("eisa_") + name, - QLatin1String("serial_") + name, - size, - QByteArray("EDID_") + name.toUtf8()); - auto mode = QSharedPointer::create(size, 60000, OutputMode::Flag::Preferred); setModesInternal({mode}, mode); m_renderLoop->setRefreshRate(mode->refreshRate()); + setInformation(Information{ + .name = QStringLiteral("Virtual-") + name, + .physicalSize = size, + .placeholder = type == Type::Placeholder, + }); + recreateSurface(); } diff --git a/src/backends/drm/drm_virtual_output.h b/src/backends/drm/drm_virtual_output.h index ef28b3f648..da81a7fcc3 100644 --- a/src/backends/drm/drm_virtual_output.h +++ b/src/backends/drm/drm_virtual_output.h @@ -24,9 +24,14 @@ class DrmPipelineLayer; class DrmVirtualOutput : public DrmAbstractOutput { Q_OBJECT + public: - DrmVirtualOutput(const QString &name, DrmGpu *gpu, const QSize &size); - DrmVirtualOutput(DrmGpu *gpu, const QSize &size); + enum class Type { + Virtual, + Placeholder, + }; + + DrmVirtualOutput(const QString &name, DrmGpu *gpu, const QSize &size, Type type); ~DrmVirtualOutput() override; bool present() override; diff --git a/src/backends/virtual/virtual_output.cpp b/src/backends/virtual/virtual_output.cpp index a9630de184..4bf4555c1c 100644 --- a/src/backends/virtual/virtual_output.cpp +++ b/src/backends/virtual/virtual_output.cpp @@ -25,7 +25,9 @@ VirtualOutput::VirtualOutput(VirtualBackend *parent) static int identifier = -1; m_identifier = ++identifier; - setName("Virtual-" + QString::number(identifier)); + setInformation(Information{ + .name = QStringLiteral("Virtual-%1").arg(identifier), + }); } VirtualOutput::~VirtualOutput() @@ -48,12 +50,6 @@ void VirtualOutput::init(const QPoint &logicalPosition, const QSize &pixelSize) m_renderLoop->setRefreshRate(refreshRate); m_vsyncMonitor->setRefreshRate(refreshRate); - initialize(QByteArray("model_").append(QByteArray::number(m_identifier)), - QByteArray("manufacturer_").append(QByteArray::number(m_identifier)), - QByteArray("eisa_").append(QByteArray::number(m_identifier)), - QByteArray("serial_").append(QByteArray::number(m_identifier)), - pixelSize, QByteArray("EDID_").append(QByteArray::number(m_identifier))); - setGeometry(QRect(logicalPosition, pixelSize)); } diff --git a/src/backends/wayland/wayland_output.cpp b/src/backends/wayland/wayland_output.cpp index f51e90dcba..7ecc120715 100644 --- a/src/backends/wayland/wayland_output.cpp +++ b/src/backends/wayland/wayland_output.cpp @@ -32,9 +32,11 @@ WaylandOutput::WaylandOutput(Surface *surface, WaylandBackend *backend) { static int identifier = -1; identifier++; - setName("WL-" + QString::number(identifier)); + setInformation(Information{ + .name = QStringLiteral("WL-%1").arg(identifier), + .capabilities = Capability::Dpms, + }); - setCapabilityInternal(Capability::Dpms); connect(surface, &Surface::frameRendered, this, [this] { m_rendered = true; Q_EMIT frameRendered(); @@ -64,9 +66,6 @@ void WaylandOutput::init(const QPoint &logicalPosition, const QSize &pixelSize) auto mode = QSharedPointer::create(pixelSize, s_refreshRate); setModesInternal({mode}, mode); - static uint i = 0; - initialize(QStringLiteral("model_%1").arg(i++), "manufacturer_TODO", "eisa_TODO", "serial_TODO", pixelSize, {}); - moveTo(logicalPosition); setScale(backend()->initialOutputScale()); } diff --git a/src/backends/x11/standalone/x11_output.cpp b/src/backends/x11/standalone/x11_output.cpp index 2ab5020940..4669a2faa4 100644 --- a/src/backends/x11/standalone/x11_output.cpp +++ b/src/backends/x11/standalone/x11_output.cpp @@ -13,10 +13,9 @@ namespace KWin { -X11Output::X11Output(const QString &name, QObject *parent) +X11Output::X11Output(QObject *parent) : Output(parent) { - setName(name); } RenderLoop *X11Output::renderLoop() const @@ -69,9 +68,4 @@ void X11Output::setMode(const QSize &size, int refreshRate) setModesInternal({mode}, mode); } -void X11Output::setPhysicalSize(const QSize &size) -{ - setPhysicalSizeInternal(size); -} - } diff --git a/src/backends/x11/standalone/x11_output.h b/src/backends/x11/standalone/x11_output.h index 4c53329ee9..c0cd1cac9f 100644 --- a/src/backends/x11/standalone/x11_output.h +++ b/src/backends/x11/standalone/x11_output.h @@ -28,7 +28,7 @@ class KWIN_EXPORT X11Output : public Output Q_OBJECT public: - explicit X11Output(const QString &name, QObject *parent = nullptr); + explicit X11Output(QObject *parent = nullptr); bool usesSoftwareCursor() const override; @@ -40,7 +40,6 @@ public: void setColorTransformation(const QSharedPointer &transformation) override; - void setPhysicalSize(const QSize &size); void setMode(const QSize &size, int refreshRate); private: diff --git a/src/backends/x11/standalone/x11_platform.cpp b/src/backends/x11/standalone/x11_platform.cpp index d67f9aacda..fd23422dbd 100644 --- a/src/backends/x11/standalone/x11_platform.cpp +++ b/src/backends/x11/standalone/x11_platform.cpp @@ -531,7 +531,7 @@ void X11StandalonePlatform::doUpdateOutputs() changed.append(output); removed.removeOne(output); } else { - output = new X11Output(outputInfo.name()); + output = new X11Output(); added.append(output); } @@ -560,7 +560,11 @@ void X11StandalonePlatform::doUpdateOutputs() case XCB_RANDR_ROTATION_REFLECT_Y: break; } - output->setPhysicalSize(physicalSize); + + output->setInformation(X11Output::Information{ + .name = outputInfo.name(), + .physicalSize = physicalSize, + }); break; } } diff --git a/src/backends/x11/standalone/x11placeholderoutput.cpp b/src/backends/x11/standalone/x11placeholderoutput.cpp index e1c4361f5c..5d69aefd7a 100644 --- a/src/backends/x11/standalone/x11placeholderoutput.cpp +++ b/src/backends/x11/standalone/x11placeholderoutput.cpp @@ -20,16 +20,12 @@ X11PlaceholderOutput::X11PlaceholderOutput(RenderLoop *loop, QObject *parent) pixelSize = QSize(screen->width_in_pixels, screen->height_in_pixels); } - const QByteArray model = QByteArrayLiteral("kwin"); - const QByteArray manufacturer = QByteArrayLiteral("xorg"); - const QByteArray eisaId; - const QByteArray serial; - - initialize(model, manufacturer, eisaId, serial, pixelSize, QByteArray()); - setName(QStringLiteral("Placeholder-0")); - auto mode = QSharedPointer::create(pixelSize, 60000); setModesInternal({mode}, mode); + + setInformation(Information{ + .name = QStringLiteral("Placeholder-0"), + }); } RenderLoop *X11PlaceholderOutput::renderLoop() const diff --git a/src/backends/x11/windowed/x11windowed_output.cpp b/src/backends/x11/windowed/x11windowed_output.cpp index 8b5d57268f..ceeb1b78de 100644 --- a/src/backends/x11/windowed/x11windowed_output.cpp +++ b/src/backends/x11/windowed/x11windowed_output.cpp @@ -35,7 +35,9 @@ X11WindowedOutput::X11WindowedOutput(X11WindowedBackend *backend) static int identifier = -1; identifier++; - setName("X11-" + QString::number(identifier)); + setInformation(Information{ + .name = QStringLiteral("X11-%1").arg(identifier), + }); connect(m_vsyncMonitor, &VsyncMonitor::vblankOccurred, this, &X11WindowedOutput::vblank); } @@ -67,10 +69,6 @@ void X11WindowedOutput::init(const QPoint &logicalPosition, const QSize &pixelSi auto mode = QSharedPointer::create(pixelSize, refreshRate); setModesInternal({mode}, mode); - // Physicial size must be adjusted, such that QPA calculates correct sizes of - // internal elements. - const QSize physicalSize = pixelSize / 96.0 * 25.4 / m_backend->initialOutputScale(); - initialize("model_TODO", "manufacturer_TODO", "eisa_TODO", "serial_TODO", physicalSize, {}); setGeometry(logicalPosition, pixelSize); setScale(m_backend->initialOutputScale()); diff --git a/src/output.cpp b/src/output.cpp index 1372aa1076..008ec17238 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -71,7 +71,7 @@ Output::~Output() QString Output::name() const { - return m_name; + return m_information.name; } QUuid Output::uuid() const @@ -86,27 +86,27 @@ Output::Transform Output::transform() const QString Output::eisaId() const { - return m_eisaId; + return m_information.eisaId; } QString Output::manufacturer() const { - return m_manufacturer; + return m_information.manufacturer; } QString Output::model() const { - return m_model; + return m_information.model; } QString Output::serialNumber() const { - return m_serialNumber; + return m_information.serialNumber; } bool Output::isInternal() const { - return m_internal; + return m_information.internal; } void Output::inhibitDirectScanout() @@ -142,15 +142,7 @@ QRect Output::mapFromGlobal(const QRect &rect) const Output::Capabilities Output::capabilities() const { - return m_capabilities; -} - -void Output::setCapabilityInternal(Capability capability, bool on) -{ - if (static_cast(m_capabilities & capability) != on) { - m_capabilities.setFlag(capability, on); - Q_EMIT capabilitiesChanged(); - } + return m_information.capabilities; } qreal Output::scale() const @@ -174,7 +166,7 @@ QRect Output::geometry() const QSize Output::physicalSize() const { - return orientateSize(m_physicalSize); + return orientateSize(m_information.physicalSize); } int Output::refreshRate() const @@ -202,7 +194,7 @@ QSize Output::pixelSize() const QByteArray Output::edid() const { - return m_edid; + return m_information.edid; } QList> Output::modes() const @@ -233,12 +225,7 @@ void Output::setModesInternal(const QList> &modes, co Output::SubPixel Output::subPixel() const { - return m_subPixel; -} - -void Output::setSubPixelInternal(SubPixel subPixel) -{ - m_subPixel = subPixel; + return m_information.subPixel; } void Output::applyChanges(const OutputConfiguration &config) @@ -272,7 +259,7 @@ void Output::setEnabled(bool enable) QString Output::description() const { - return m_manufacturer + ' ' + m_model; + return manufacturer() + ' ' + model(); } void Output::setCurrentModeInternal(const QSharedPointer ¤tMode) @@ -295,17 +282,10 @@ static QUuid generateOutputId(const QString &eisaId, const QString &model, return QUuid::createUuidV5(kwinNs, payload); } -void Output::initialize(const QString &model, const QString &manufacturer, - const QString &eisaId, const QString &serialNumber, - const QSize &physicalSize, const QByteArray &edid) +void Output::setInformation(const Information &information) { - m_serialNumber = serialNumber; - m_eisaId = eisaId; - m_manufacturer = manufacturer.isEmpty() ? i18n("unknown") : manufacturer; - m_model = model; - m_physicalSize = physicalSize; - m_edid = edid; - m_uuid = generateOutputId(m_eisaId, m_model, m_serialNumber, m_name); + m_information = information; + m_uuid = generateOutputId(eisaId(), model(), serialNumber(), name()); } QSize Output::orientateSize(const QSize &size) const @@ -402,7 +382,7 @@ uint32_t Output::overscan() const void Output::setVrrPolicy(RenderLoop::VrrPolicy policy) { - if (renderLoop()->vrrPolicy() != policy && (m_capabilities & Capability::Vrr)) { + if (renderLoop()->vrrPolicy() != policy && (capabilities() & Capability::Vrr)) { renderLoop()->setVrrPolicy(policy); Q_EMIT vrrPolicyChanged(); } @@ -415,12 +395,7 @@ RenderLoop::VrrPolicy Output::vrrPolicy() const bool Output::isPlaceholder() const { - return m_isPlaceholder; -} - -void Output::setPlaceholder(bool isPlaceholder) -{ - m_isPlaceholder = isPlaceholder; + return m_information.placeholder; } Output::RgbRange Output::rgbRange() const @@ -436,11 +411,6 @@ void Output::setRgbRangeInternal(RgbRange range) } } -void Output::setPhysicalSizeInternal(const QSize &size) -{ - m_physicalSize = size; -} - void Output::setColorTransformation(const QSharedPointer &transformation) { Q_UNUSED(transformation); diff --git a/src/output.h b/src/output.h index afad24a3b2..c4e373c54f 100644 --- a/src/output.h +++ b/src/output.h @@ -297,18 +297,22 @@ Q_SIGNALS: void rgbRangeChanged(); protected: - void initialize(const QString &model, const QString &manufacturer, - const QString &eisaId, const QString &serialNumber, - const QSize &physicalSize, const QByteArray &edid); + struct Information + { + QString name; + QString manufacturer; + QString model; + QString serialNumber; + QString eisaId; + QSize physicalSize; + QByteArray edid; + SubPixel subPixel = SubPixel::Unknown; + Capabilities capabilities; + bool internal = false; + bool placeholder = false; + }; - void setName(const QString &name) - { - m_name = name; - } - void setInternal(bool set) - { - m_internal = set; - } + void setInformation(const Information &information); virtual void updateEnablement(bool enable) { @@ -319,12 +323,8 @@ protected: void setCurrentModeInternal(const QSharedPointer ¤tMode); void setTransformInternal(Transform transform); void setDpmsModeInternal(DpmsMode dpmsMode); - void setCapabilityInternal(Capability capability, bool on = true); - void setSubPixelInternal(SubPixel subPixel); void setOverscanInternal(uint32_t overscan); - void setPlaceholder(bool isPlaceholder); void setRgbRangeInternal(RgbRange range); - void setPhysicalSizeInternal(const QSize &size); QSize orientateSize(const QSize &size) const; @@ -332,25 +332,16 @@ private: Q_DISABLE_COPY(Output) EffectScreenImpl *m_effectScreen = nullptr; int m_directScanoutCount = 0; - QString m_name; - QString m_eisaId; - QString m_manufacturer; - QString m_model; - QString m_serialNumber; + Information m_information; QUuid m_uuid; - QSize m_physicalSize; QPoint m_position; qreal m_scale = 1; - Capabilities m_capabilities; Transform m_transform = Transform::Normal; - QByteArray m_edid; QList> m_modes; QSharedPointer m_currentMode; DpmsMode m_dpmsMode = DpmsMode::On; SubPixel m_subPixel = SubPixel::Unknown; bool m_isEnabled = true; - bool m_internal = false; - bool m_isPlaceholder = false; uint32_t m_overscan = 0; RgbRange m_rgbRange = RgbRange::Automatic; friend class EffectScreenImpl; // to access m_effectScreen