Commit af609c37 authored by Roman Gilg's avatar Roman Gilg

Wayland: when blocking remember latest config change instead of crashing

Summary:
Configuration change requests by clients while the Wayland server is
processing a change lead to a failing assert in blockSignals().

Instead in such a situation remember the latest configuration change request
and apply it after the current change has been processed.

Test Plan:
Auto test exposing the problem. Also manually with my output color correction
code.

Reviewers: #plasma, davidedmundson

Reviewed By: #plasma, davidedmundson

Subscribers: apol, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D12517
parent feb1099c
......@@ -53,6 +53,7 @@ private Q_SLOTS:
void testRotationChange_data();
void testScaleChange();
void testModeChange();
void testApplyOnPending();
private:
......@@ -265,6 +266,60 @@ void TestKWaylandConfig::testModeChange()
QCOMPARE(configSpy.count(), 1);
}
void TestKWaylandConfig::testApplyOnPending()
{
auto op = new GetConfigOperation();
QVERIFY(op->exec());
auto config = op->config();
QVERIFY(config);
auto op2 = new GetConfigOperation();
QVERIFY(op2->exec());
auto config2 = op2->config();
QVERIFY(config2);
KScreen::ConfigMonitor *monitor = KScreen::ConfigMonitor::instance();
monitor->addConfig(config);
QSignalSpy configSpy(monitor, &KScreen::ConfigMonitor::configurationChanged);
auto output = config->outputs()[1]; // is this id stable enough?
QCOMPARE(output->scale(), 1.0);
output->setScale(2);
QSignalSpy serverSpy(m_server, &WaylandTestServer::configChanged);
QSignalSpy serverReceivedSpy(m_server, &WaylandTestServer::configReceived);
m_server->suspendChanges(true);
new SetConfigOperation(config, this);
/* Apply next config */
auto output2 = config2->outputs()[2]; // is this id stable enough?
QCOMPARE(output2->scale(), 2.0);
output2->setScale(3);
new SetConfigOperation(config2, this);
QVERIFY(serverReceivedSpy.wait());
QCOMPARE(serverReceivedSpy.count(), 1);
m_server->suspendChanges(false);
QVERIFY(configSpy.wait());
// check if the server changed
QCOMPARE(serverSpy.count(), 1);
QCOMPARE(configSpy.count(), 1);
QCOMPARE(output->scale(), 2);
QCOMPARE(output2->scale(), 3);
QVERIFY(configSpy.wait());
// check if the server changed
QCOMPARE(serverSpy.count(), 2);
QCOMPARE(configSpy.count(), 2);
QCOMPARE(output2->scale(), 3);
}
QTEST_GUILESS_MAIN(TestKWaylandConfig)
......
......@@ -48,6 +48,7 @@ WaylandConfig::WaylandConfig(QObject *parent)
, m_blockSignals(true)
, m_newOutputId(0)
, m_kscreenConfig(nullptr)
, m_kscreenPendingConfig(nullptr)
, m_screen(new WaylandScreen(this))
{
connect(this, &WaylandConfig::initialized, &m_syncLoop, &QEventLoop::quit);
......@@ -274,6 +275,15 @@ QMap<int, WaylandOutput*> WaylandConfig::outputMap() const
return m_outputMap;
}
void WaylandConfig::tryPendingConfig()
{
if (!m_kscreenPendingConfig) {
return;
}
applyConfig(m_kscreenPendingConfig);
m_kscreenPendingConfig = nullptr;
}
void WaylandConfig::applyConfig(const KScreen::ConfigPtr &newConfig)
{
using namespace KWayland::Client;
......@@ -281,6 +291,12 @@ void WaylandConfig::applyConfig(const KScreen::ConfigPtr &newConfig)
auto wlOutputConfiguration = m_outputManagement->createConfiguration();
bool changed = false;
if (m_blockSignals) {
/* Last apply still pending, remember new changes and apply afterwards */
m_kscreenPendingConfig = newConfig;
return;
}
Q_FOREACH (auto output, newConfig->outputs()) {
auto o_old = m_outputMap[output->id()];
auto device = o_old->outputDevice();
......@@ -334,11 +350,13 @@ void WaylandConfig::applyConfig(const KScreen::ConfigPtr &newConfig)
wlOutputConfiguration->deleteLater();
unblockSignals();
Q_EMIT configChanged(toKScreenConfig());
tryPendingConfig();
});
connect(wlOutputConfiguration, &OutputConfiguration::failed, this, [this, wlOutputConfiguration] {
wlOutputConfiguration->deleteLater();
unblockSignals();
Q_EMIT configChanged(toKScreenConfig());
tryPendingConfig();
});
blockSignals();
// Now ask the compositor to apply the changes
......
......@@ -92,6 +92,7 @@ private:
void initConnection();
void blockSignals();
void unblockSignals();
void tryPendingConfig();
KWayland::Client::ConnectionThread *m_connection;
KWayland::Client::EventQueue *m_queue;
......@@ -111,6 +112,7 @@ private:
QEventLoop m_syncLoop;
int m_newOutputId;
KScreen::ConfigPtr m_kscreenConfig;
KScreen::ConfigPtr m_kscreenPendingConfig;
WaylandScreen *m_screen;
};
......
......@@ -19,7 +19,6 @@
#include "waylandtestserver.h"
#include "waylandconfigreader.h"
#include <QDebug>
#include <QJsonArray>
#include <QJsonDocument>
......@@ -41,6 +40,8 @@ WaylandTestServer::WaylandTestServer(QObject *parent)
, m_display(nullptr)
, m_outputManagement(nullptr)
, m_dpmsManager(nullptr)
, m_suspendChanges(false)
, m_waiting(nullptr)
{
}
......@@ -106,9 +107,23 @@ QList<KWayland::Server::OutputDeviceInterface*> WaylandTestServer::outputs() con
return m_outputs;
}
void WaylandTestServer::suspendChanges(bool suspend)
{
if (m_suspendChanges == suspend) {
return;
}
m_suspendChanges = suspend;
if (!suspend && m_waiting) {
m_waiting->setApplied();
m_waiting = nullptr;
Q_EMIT configChanged();
}
}
void WaylandTestServer::configurationChangeRequested(KWayland::Server::OutputConfigurationInterface* configurationInterface)
{
qCDebug(KSCREEN_WAYLAND_TESTSERVER) << "Server received change request, changes:" << configurationInterface->changes().count();
Q_EMIT configReceived();
auto changes = configurationInterface->changes();
Q_FOREACH (const auto &outputdevice, changes.keys()) {
......@@ -135,6 +150,12 @@ void WaylandTestServer::configurationChangeRequested(KWayland::Server::OutputCon
}
}
if (m_suspendChanges) {
Q_ASSERT(!m_waiting);
m_waiting = configurationInterface;
return;
}
configurationInterface->setApplied();
//showOutputs();
Q_EMIT configChanged();
......
......@@ -59,11 +59,14 @@ public:
int outputCount() const;
void suspendChanges(bool suspend);
Q_SIGNALS:
void outputsChanged();
void started();
void configReceived();
void configChanged();
private Q_SLOTS:
......@@ -76,6 +79,8 @@ private:
QList<KWayland::Server::OutputDeviceInterface*> m_outputs;
KWayland::Server::OutputManagementInterface *m_outputManagement;
KWayland::Server::DpmsManagerInterface *m_dpmsManager;
bool m_suspendChanges;
KWayland::Server::OutputConfigurationInterface *m_waiting;
};
} // namespace
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment