From 7e095412aacf72b57983f84de91aa39d80d9e2ba Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Mon, 22 Jan 2024 23:10:30 +0100 Subject: [PATCH] outputconfigurationstore: add a fallback for when edid parsing fails When edid parsing fails, KWin will base output settings on the connector, which isn't great on its own, but at least works in many cases. When the edid can be parsed later though, the display settings will reset because now the edid identifier is used to exclude the old config (in which the latter is missing). To work around that, this commit adds output identification based on the edid hash, which is also not ideal, but can be safely matched with in case no output config with a matching edid identifier exists. --- src/outputconfigurationstore.cpp | 30 +++++++++++++++++++++++++++++- src/outputconfigurationstore.h | 1 + 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/outputconfigurationstore.cpp b/src/outputconfigurationstore.cpp index 142bc35a23..028850e218 100644 --- a/src/outputconfigurationstore.cpp +++ b/src/outputconfigurationstore.cpp @@ -133,10 +133,13 @@ std::optional OutputConfigurationStore::findOutput(Output *output, const const bool uniqueEdid = !output->edid().identifier().isEmpty() && std::none_of(allOutputs.begin(), allOutputs.end(), [output](Output *otherOutput) { return otherOutput != output && otherOutput->edid().identifier() == output->edid().identifier(); }); + const bool uniqueEdidHash = !output->edid().hash().isEmpty() && std::none_of(allOutputs.begin(), allOutputs.end(), [output](Output *otherOutput) { + return otherOutput != output && otherOutput->edid().hash() == output->edid().hash(); + }); const bool uniqueMst = !output->mstPath().isEmpty() && std::none_of(allOutputs.begin(), allOutputs.end(), [output](Output *otherOutput) { return otherOutput != output && otherOutput->edid().identifier() == output->edid().identifier() && otherOutput->mstPath() == output->mstPath(); }); - const auto it = std::find_if(m_outputs.begin(), m_outputs.end(), [uniqueEdid, uniqueMst, output](const auto &outputState) { + auto it = std::find_if(m_outputs.begin(), m_outputs.end(), [&](const auto &outputState) { if (output->edid().isValid()) { if (outputState.edidIdentifier != output->edid().identifier()) { return false; @@ -144,6 +147,13 @@ std::optional OutputConfigurationStore::findOutput(Output *output, const return true; } } + if (!output->edid().hash().isEmpty()) { + if (outputState.edidHash != output->edid().hash()) { + return false; + } else if (uniqueEdidHash) { + return true; + } + } if (outputState.mstPath != output->mstPath()) { return false; } else if (uniqueMst) { @@ -151,6 +161,12 @@ std::optional OutputConfigurationStore::findOutput(Output *output, const } return outputState.connectorName == output->name(); }); + if (it == m_outputs.end() && uniqueEdidHash) { + // handle the edge case of EDID parsing failing in the past but not failing anymore + it = std::find_if(m_outputs.begin(), m_outputs.end(), [&](const auto &outputState) { + return outputState.edidHash == output->edid().hash(); + }); + } if (it != m_outputs.end()) { return std::distance(m_outputs.begin(), it); } else { @@ -197,6 +213,7 @@ void OutputConfigurationStore::storeConfig(const QList &allOutputs, bo m_outputs[*outputIndex] = OutputState{ .edidIdentifier = output->edid().identifier(), .connectorName = output->name(), + .edidHash = output->edid().isValid() ? output->edid().hash() : QString{}, .mstPath = output->mstPath(), .mode = ModeData{ .size = mode->size(), @@ -229,6 +246,7 @@ void OutputConfigurationStore::storeConfig(const QList &allOutputs, bo m_outputs[*outputIndex] = OutputState{ .edidIdentifier = output->edid().identifier(), .connectorName = output->name(), + .edidHash = output->edid().isValid() ? output->edid().hash() : QString{}, .mstPath = output->mstPath(), .mode = ModeData{ .size = mode->size(), @@ -554,6 +572,12 @@ void OutputConfigurationStore::load() hasIdentifier = true; } } + if (const auto it = data.find("edidHash"); it != data.end()) { + if (const auto str = it->toString(); !str.isEmpty()) { + state.edidHash = str; + hasIdentifier = true; + } + } if (const auto it = data.find("connectorName"); it != data.end()) { if (const auto str = it->toString(); !str.isEmpty()) { state.connectorName = str; @@ -576,6 +600,7 @@ void OutputConfigurationStore::load() const bool hasDuplicate = std::any_of(outputDatas.begin(), outputDatas.end(), [&state](const auto &data) { return data && data->edidIdentifier == state.edidIdentifier + && data->edidHash == state.edidHash && data->mstPath == state.mstPath && data->connectorName == state.connectorName; }); @@ -808,6 +833,9 @@ void OutputConfigurationStore::save() if (output.edidIdentifier) { o["edidIdentifier"] = *output.edidIdentifier; } + if (!output.edidHash.isEmpty()) { + o["edidHash"] = output.edidHash; + } if (output.connectorName) { o["connectorName"] = *output.connectorName; } diff --git a/src/outputconfigurationstore.h b/src/outputconfigurationstore.h index 6386339764..b194d10539 100644 --- a/src/outputconfigurationstore.h +++ b/src/outputconfigurationstore.h @@ -62,6 +62,7 @@ private: std::optional edidIdentifier; std::optional connectorName; // empty if invalid + QString edidHash; QString mstPath; // actual state std::optional mode;