diff --git a/common/control.h b/common/control.h --- a/common/control.h +++ b/common/control.h @@ -32,8 +32,6 @@ virtual ~Control() = default; - static OutputRetention getOutputRetention(const QString &outputId, const QMap &retentions); - virtual QString filePath() = 0; protected: @@ -49,13 +47,16 @@ public: ControlConfig(KScreen::ConfigPtr config); - QMap readInOutputRetentionValues(); + OutputRetention getOutputRetention(const KScreen::OutputPtr &output) const; + OutputRetention getOutputRetention(const QString &outputId, const QString &outputName) const; QString filePath() override; static QString filePath(const QString &hash); private: KScreen::ConfigPtr m_config; + QVariantList m_info; + QStringList m_duplicateOutputIds; }; class ControlOutput : public Control diff --git a/common/control.cpp b/common/control.cpp --- a/common/control.cpp +++ b/common/control.cpp @@ -31,18 +31,52 @@ return Globals::dirPath() % s_dirName; } -Control::OutputRetention Control::getOutputRetention(const QString &outputId, const QMap &retentions) +Control::OutputRetention Control::convertVariantToOutputRetention(QVariant variant) { - if (retentions.contains(outputId)) { - return retentions[outputId]; + if (variant.canConvert()) { + const auto retention = variant.toInt(); + if (retention == (int)OutputRetention::Global) { + return OutputRetention::Global; + } + if (retention == (int)OutputRetention::Individual) { + return OutputRetention::Individual; + } } - // info for output not found return OutputRetention::Undefined; } ControlConfig::ControlConfig(KScreen::ConfigPtr config) : m_config(config) { +// qDebug() << "Looking for control file:" << config->connectedOutputsHash(); + QFile file(filePath(config->connectedOutputsHash())); + if (file.open(QIODevice::ReadOnly)) { + QJsonDocument parser; + m_info = parser.fromJson(file.readAll()).toVariant().toList(); + } + + // TODO: use a file watcher in case of changes to the control file while + // object exists? + + // As global outputs are indexed by a hash of their edid, which is not unique, + // to be able to tell apart multiple identical outputs, these need special treatment + { + QStringList allIds; + const auto outputs = config->outputs(); + allIds.reserve(outputs.count()); + for (const KScreen::OutputPtr &output : outputs) { + const auto outputId = output->hash(); + if (allIds.contains(outputId) && !m_duplicateOutputIds.contains(outputId)) { + m_duplicateOutputIds << outputId; + } + allIds << outputId; + } + } + + // TODO: this is same in Output::readInOutputs of the daemon. Combine? + + // TODO: connect to outputs added/removed signals and reevaluate duplicate ids + // in case of such a change while object exists? } QString ControlConfig::filePath(const QString &hash) @@ -62,46 +96,38 @@ return ControlConfig::filePath(m_config->connectedOutputsHash()); } -Control::OutputRetention Control::convertVariantToOutputRetention(QVariant variant) +Control::OutputRetention ControlConfig::getOutputRetention(const KScreen::OutputPtr &output) const { - if (variant.canConvert()) { - const auto retention = variant.toInt(); - if (retention == (int)OutputRetention::Global) { - return OutputRetention::Global; - } - if (retention == (int)OutputRetention::Individual) { - return OutputRetention::Individual; - } - } - return OutputRetention::Undefined; + return getOutputRetention(output->hash(), output->name()); } -QMap ControlConfig::readInOutputRetentionValues() +Control::OutputRetention ControlConfig::getOutputRetention(const QString &outputId, const QString &outputName) const { -// qDebug() << "Looking for control file:" << m_config->connectedOutputsHash(); - QFile file(filePath(m_config->connectedOutputsHash())); - if (!file.open(QIODevice::ReadOnly)) { - // TODO: have a logging category -// qCDebug(KSCREEN_COMMON) << "Failed to open file" << file.fileName(); - return QMap(); - } - - QJsonDocument parser; - const QVariantList outputsInfo = parser.fromJson(file.readAll()).toVariant().toList(); - QMap retentions; - - for (const auto variantInfo : outputsInfo) { + for (const auto variantInfo : m_info) { const QVariantMap info = variantInfo.toMap(); - // TODO: this does not yet consider the output name (i.e. connector). Necessary? - const QString outputHash = info[QStringLiteral("id")].toString(); - if (outputHash.isEmpty()) { + const QString outputIdInfo = info[QStringLiteral("id")].toString(); + if (outputIdInfo.isEmpty()) { + continue; + } + if (outputId != outputIdInfo) { continue; } - retentions[outputHash] = convertVariantToOutputRetention(info[QStringLiteral("retention")]); - } - return retentions; + if (!outputName.isEmpty() && m_duplicateOutputIds.contains(outputId)) { + // We may have identical outputs connected, these will have the same id in the config + // in order to find the right one, also check the output's name (usually the connector) + const auto metadata = info[QStringLiteral("metadata")].toMap(); + const auto outputNameInfo = metadata[QStringLiteral("name")].toString(); + if (outputName != outputNameInfo) { + // was a duplicate id, but info not for this output + continue; + } + } + return convertVariantToOutputRetention(info[QStringLiteral("retention")]); + } + // info for output not found + return OutputRetention::Undefined; } ControlOutput::ControlOutput(KScreen::OutputPtr output) diff --git a/kded/config.cpp b/kded/config.cpp --- a/kded/config.cpp +++ b/kded/config.cpp @@ -114,7 +114,7 @@ QJsonDocument parser; QVariantList outputs = parser.fromJson(file.readAll()).toVariant().toList(); - Output::readInOutputs(config->data()->outputs(), outputs, ControlConfig(config->data()).readInOutputRetentionValues()); + Output::readInOutputs(config->data(), outputs); QSize screenSize; for (const auto &output : config->data()->outputs()) { @@ -170,7 +170,7 @@ } const KScreen::OutputList outputs = m_data->outputs(); - const auto retentions = ControlConfig(m_data).readInOutputRetentionValues(); + const auto control = ControlConfig(m_data); QVariantList outputList; Q_FOREACH(const KScreen::OutputPtr &output, outputs) { @@ -191,7 +191,7 @@ pos[QStringLiteral("y")] = output->pos().y(); info[QStringLiteral("pos")] = pos; - if (Control::getOutputRetention(output->hash(), retentions) != Control::OutputRetention::Individual) { + if (control.getOutputRetention(output->hash(), output->name()) != Control::OutputRetention::Individual) { // try to update global output data Output::writeGlobal(output); } diff --git a/kded/output.h b/kded/output.h --- a/kded/output.h +++ b/kded/output.h @@ -26,7 +26,7 @@ class Output { public: - static void readInOutputs(KScreen::OutputList outputs, const QVariantList &outputsInfo, const QMap &retentions); + static void readInOutputs(KScreen::ConfigPtr config, const QVariantList &outputsInfo); static void writeGlobal(const KScreen::OutputPtr &output); static bool writeGlobalPart(const KScreen::OutputPtr &output, QVariantMap &info); diff --git a/kded/output.cpp b/kded/output.cpp --- a/kded/output.cpp +++ b/kded/output.cpp @@ -130,8 +130,10 @@ readInGlobalPartFromInfo(output, info); } -void Output::readInOutputs(KScreen::OutputList outputs, const QVariantList &outputsInfo, const QMap &retentions) +void Output::readInOutputs(KScreen::ConfigPtr config, const QVariantList &outputsInfo) { + KScreen::OutputList outputs = config->outputs(); + ControlConfig control(config); // As global outputs are indexed by a hash of their edid, which is not unique, // to be able to tell apart multiple identical outputs, these need special treatment QStringList duplicateIds; @@ -154,7 +156,6 @@ continue; } const auto outputId = output->hash(); - const auto retention = Control::getOutputRetention(outputId, retentions); bool infoFound = false; for (const auto &variantInfo : outputsInfo) { const QVariantMap info = variantInfo.toMap(); @@ -171,9 +172,8 @@ continue; } } - infoFound = true; - readIn(output, info, retention); + readIn(output, info, control.getOutputRetention(output)); break; } if (!infoFound) { diff --git a/tests/kded/configtest.cpp b/tests/kded/configtest.cpp --- a/tests/kded/configtest.cpp +++ b/tests/kded/configtest.cpp @@ -34,6 +34,7 @@ Q_OBJECT private Q_SLOTS: + void init(); void initTestCase(); void testSimpleConfig(); @@ -101,10 +102,14 @@ return configWrapper; } +void TestConfig::init() +{ + Globals::setDirPath(QStringLiteral(TEST_DATA "serializerdata/")); +} + void TestConfig::initTestCase() { qputenv("KSCREEN_LOGGING", "false"); - Globals::setDirPath(QStringLiteral(TEST_DATA "serializerdata/")); } void TestConfig::testSimpleConfig() @@ -481,8 +486,6 @@ QCOMPARE(output2->name(), QLatin1String("OUTPUT-2")); QCOMPARE(output2->isEnabled(), true); QCOMPARE(output2->isPrimary(), false); - - Globals::setDirPath(QStringLiteral(TEST_DATA "/serializerdata/")); } void TestConfig::testFixedConfig() @@ -505,8 +508,6 @@ // Check if both files exist QFile fixedCfg(fixedCfgPath); QVERIFY(fixedCfg.exists()); - - Globals::setDirPath(QStringLiteral(TEST_DATA "/serializerdata/")); } diff --git a/tests/kded/serializerdata/control/configs/e919cc0dd7aea8d8f519bdf8b93a6f69 b/tests/kded/serializerdata/control/configs/e919cc0dd7aea8d8f519bdf8b93a6f69 --- a/tests/kded/serializerdata/control/configs/e919cc0dd7aea8d8f519bdf8b93a6f69 +++ b/tests/kded/serializerdata/control/configs/e919cc0dd7aea8d8f519bdf8b93a6f69 @@ -1,26 +1,44 @@ [ { "retention" : 1, - "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0" + "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0", + "metadata" : { + "name" : "DisplayPort-0" + } }, { "retention" : 1, - "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0" + "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0", + "metadata" : { + "name" : "DisplayPort-1" + } }, { "retention" : 1, - "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0" + "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0", + "metadata" : { + "name" : "DisplayPort-2" + } }, { "retention" : 1, - "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0" + "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0", + "metadata" : { + "name" : "DisplayPort-3" + } }, { "retention" : 1, - "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0" + "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0", + "metadata" : { + "name" : "DVI-0" + } }, { "retention" : 1, - "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0" + "id" : "be55eeb5fcc1e775f321c1ae3aa02ef0", + "metadata" : { + "name" : "DVI-1" + } } ]