From ee0dd8365881c707d7f846cdaf5d63dec269aa5d Mon Sep 17 00:00:00 2001 From: reionwong Date: Thu, 8 Oct 2026 13:02:57 +0800 Subject: [PATCH] feat(screen): preview mode changes with keep and revert --- screen/com.cutefish.DisplayConfig1.xml | 6 ++++ screen/displayclient.cpp | 12 ++++--- screen/displayclient.h | 5 ++- screen/outputmodel.cpp | 14 ++++++++ screen/outputmodel.h | 4 +++ screen/screen.cpp | 3 +- screen/tests/outputmodel_test.cpp | 44 ++++++++++++++++++++++++ screen/tests/screen_integration_test.cpp | 23 ++++++++++++- 8 files changed, 104 insertions(+), 7 deletions(-) diff --git a/screen/com.cutefish.DisplayConfig1.xml b/screen/com.cutefish.DisplayConfig1.xml index a6e7646..1504d3a 100644 --- a/screen/com.cutefish.DisplayConfig1.xml +++ b/screen/com.cutefish.DisplayConfig1.xml @@ -28,6 +28,12 @@ Adapted for Cutefish: a versioned session D-Bus interface without Ash or Mojo. + + + + + + diff --git a/screen/displayclient.cpp b/screen/displayclient.cpp index 5807a76..b499833 100644 --- a/screen/displayclient.cpp +++ b/screen/displayclient.cpp @@ -139,7 +139,7 @@ void DisplayClient::refresh() QString DisplayClient::apply(const QList &displays, const QString &primary, const QList &placements, - const ipc::MirrorGroups &mirrorGroups) + const ipc::MirrorGroups &mirrorGroups, bool preview) { if (!available() || operationPending()) return {}; @@ -149,9 +149,13 @@ QString DisplayClient::apply(const QList &displays, const Q m_operationTimeout.start(); const auto epoch = m_serviceEpoch; const auto operation = ++m_operationSerial; - auto pending = m_interface->SetConfiguration(m_state.instanceId, m_state.topologyGeneration, - m_state.appliedRevision, displays, primary, - placements, mirrorGroups); + auto pending = preview + ? m_interface->ApplyConfiguration(m_state.instanceId, m_state.topologyGeneration, + m_state.appliedRevision, displays, primary, + placements, mirrorGroups) + : m_interface->SetConfiguration(m_state.instanceId, m_state.topologyGeneration, + m_state.appliedRevision, displays, primary, + placements, mirrorGroups); auto *watcher = new QDBusPendingCallWatcher(pending, this); connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, watcher, epoch, operation] { QDBusPendingReply reply = *watcher; diff --git a/screen/displayclient.h b/screen/displayclient.h index 8c593e5..3804769 100644 --- a/screen/displayclient.h +++ b/screen/displayclient.h @@ -51,10 +51,13 @@ public: const State &state() const { return m_state; } void refresh(); + // A preview is applied but persisted only by confirm(); the compositor + // reverts it on its own deadline otherwise. QString apply(const QList &displays, const QString &primary, const QList &placements, - const cutefish::display::ipc::MirrorGroups &mirrorGroups); + const cutefish::display::ipc::MirrorGroups &mirrorGroups, + bool preview = false); void confirm(const QString &token); void revert(const QString &token); diff --git a/screen/outputmodel.cpp b/screen/outputmodel.cpp index d081dc3..1c18dff 100644 --- a/screen/outputmodel.cpp +++ b/screen/outputmodel.cpp @@ -439,6 +439,20 @@ void DisplayOutputModel::discard() setState(m_applied); } +bool DisplayOutputModel::needsConfirmation() const +{ + for (const auto &e : m_entries) { + const auto *mode = selectedMode(e); + if (!e.enabled || !mode || mode->current) + continue; + // A display being switched on has no current mode to leave behind. + for (const auto &applied : e.physical.modes) + if (applied.current) + return true; + } + return false; +} + QList DisplayOutputModel::requests() const { QList r; diff --git a/screen/outputmodel.h b/screen/outputmodel.h index 39da99f..a92cb6f 100644 --- a/screen/outputmodel.h +++ b/screen/outputmodel.h @@ -57,6 +57,10 @@ public: bool dirty() const { return m_dirty; } + // A display switched to another mode may show nothing at all, so that + // draft is previewed and kept only on confirmation. + bool needsConfirmation() const; + QList requests() const; QList placements() const; cutefish::display::ipc::MirrorGroups mirrorGroups() const; diff --git a/screen/screen.cpp b/screen/screen.cpp index 31cb48c..fb3c3f9 100644 --- a/screen/screen.cpp +++ b/screen/screen.cpp @@ -76,7 +76,8 @@ void DisplayScreen::apply() m_error.clear(); emit stateChanged(); if (m_client.apply(m_outputModel->requests(), m_outputModel->primary(), - m_outputModel->placements(), m_outputModel->mirrorGroups()) + m_outputModel->placements(), m_outputModel->mirrorGroups(), + m_outputModel->needsConfirmation()) .isEmpty()) { m_busy = false; m_error = tr("The display service is unavailable."); diff --git a/screen/tests/outputmodel_test.cpp b/screen/tests/outputmodel_test.cpp index 553a211..ee540b0 100644 --- a/screen/tests/outputmodel_test.cpp +++ b/screen/tests/outputmodel_test.cpp @@ -72,6 +72,50 @@ private slots: QCOMPARE(model.data(selected, DisplayOutputModel::IdRole).toString(), QString("1")); } + void onlyModeChangesNeedConfirmation() + { + DisplayClient::State state = extendedDisplays(2); + state.physical[0].connectionType = 1; + for (auto &physical : state.physical) { + cutefish::display::ipc::Mode mode; + mode.width = 1280; + mode.height = 720; + mode.refreshMilliHz = 60000; + physical.modes.append(mode); + } + DisplayOutputModel model; + model.setState(state); + QVERIFY(!model.needsConfirmation()); + + QVERIFY(model.setData(model.index(1), 1.25, DisplayOutputModel::ScaleRole)); + QVERIFY(model.dirty()); + QVERIFY(!model.needsConfirmation()); + + // The internal panel is no exception. + QVERIFY(model.setData(model.index(0), 1, DisplayOutputModel::ResolutionIndexRole)); + QVERIFY(model.needsConfirmation()); + QVERIFY(model.setData(model.index(0), 0, DisplayOutputModel::ResolutionIndexRole)); + QVERIFY(!model.needsConfirmation()); + + QVERIFY(model.setData(model.index(1), 1, DisplayOutputModel::ResolutionIndexRole)); + QVERIFY(model.needsConfirmation()); + + QVERIFY(model.setData(model.index(1), false, DisplayOutputModel::EnabledRole)); + QVERIFY(!model.needsConfirmation()); + + model.discard(); + QVERIFY(!model.needsConfirmation()); + + // Switching a display on picks a mode without one to fall back to. + for (auto &mode : state.physical[1].modes) + mode.current = false; + state.logical.removeLast(); + model.setState(state); + QVERIFY(model.setData(model.index(1), true, DisplayOutputModel::EnabledRole)); + QVERIFY(model.dirty()); + QVERIFY(!model.needsConfirmation()); + } + void mirroringThePrimaryHandsItToTheSource() { DisplayOutputModel model; diff --git a/screen/tests/screen_integration_test.cpp b/screen/tests/screen_integration_test.cpp index 1dcb740..eac4544 100644 --- a/screen/tests/screen_integration_test.cpp +++ b/screen/tests/screen_integration_test.cpp @@ -116,7 +116,7 @@ private slots: } } - void clickAppliesAndPersistsWithoutConfirmation() + void clickAppliesAndAModeChangeIsConfirmed() { QTemporaryDir directory; QVERIFY(directory.isValid()); @@ -161,13 +161,34 @@ private slots: QVERIFY2(screen.error().isEmpty(), qPrintable(screen.error())); QTRY_COMPARE(observer.state().logical.first().width, 1280); QTRY_COMPARE(observer.state().logical.first().height, 720); + // A mode change is previewed and kept only on confirmation. QVERIFY(model->setData(model->index(0), 2, DisplayOutputModel::ResolutionIndexRole)); QTRY_VERIFY_WITH_TIMEOUT(!screen.busy(), 5000); QVERIFY2(screen.error().isEmpty(), qPrintable(screen.error())); QTRY_COMPARE(observer.state().logical.first().width, 854); QTRY_COMPARE(observer.state().logical.first().height, 480); + QTRY_VERIFY(screen.pendingConfirmation()); + QVERIFY(screen.confirmationSeconds() > 0); + QVERIFY(!screen.dirty()); + screen.keep(); + QTRY_VERIFY_WITH_TIMEOUT(!screen.pendingConfirmation(), 5000); + QCOMPARE(observer.state().logical.first().width, 854); QCOMPARE(model->data(model->index(0), DisplayOutputModel::SizeRole).toSize(), QSize(1280, 720)); + + // Reverting restores the kept mode without a round trip through the draft. + QVERIFY(model->setData(model->index(0), 1, DisplayOutputModel::ResolutionIndexRole)); + QTRY_VERIFY_WITH_TIMEOUT(!screen.busy(), 5000); + QVERIFY2(screen.error().isEmpty(), qPrintable(screen.error())); + QTRY_VERIFY(screen.pendingConfirmation()); + QVERIFY(model->data(model->index(0), DisplayOutputModel::SizeRole).toSize() + != QSize(1280, 720)); + screen.revert(); + QTRY_VERIFY_WITH_TIMEOUT(!screen.pendingConfirmation(), 5000); + QTRY_COMPARE(model->data(model->index(0), DisplayOutputModel::SizeRole).toSize(), + QSize(1280, 720)); + QTRY_COMPARE(observer.state().logical.first().width, 854); + QVERIFY(!screen.dirty()); int currentModes = 0; for (const auto &mode : observer.state().physical.first().modes) { if (!mode.current)