commit: e8ecf8e32790fec99329c16cb2be96ce4f5ff513 Author: Andreas Sturmlechner <andreas.sturmlechner <AT> gmail <DOT> com> AuthorDate: Thu Jun 23 17:16:16 2016 +0000 Commit: Michael Palimaka <kensington <AT> gentoo <DOT> org> CommitDate: Sat Jun 25 18:30:28 2016 +0000 URL: https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=e8ecf8e3
kde-plasma/kscreen: Add fix for multiscreen config persistance Reported-by: gorg86 (forums.gentoo.org) https://forums.gentoo.org/viewtopic-t-1046956.html Upstream bug: https://bugs.kde.org/show_bug.cgi?id=346961 Package-Manager: portage-2.2.28 .../kscreen/files/kscreen-5.6.5-config-fix.patch | 159 +++++++++++++++++++++ kde-plasma/kscreen/kscreen-5.6.5-r1.ebuild | 40 ++++++ 2 files changed, 199 insertions(+) diff --git a/kde-plasma/kscreen/files/kscreen-5.6.5-config-fix.patch b/kde-plasma/kscreen/files/kscreen-5.6.5-config-fix.patch new file mode 100644 index 0000000..56474aa --- /dev/null +++ b/kde-plasma/kscreen/files/kscreen-5.6.5-config-fix.patch @@ -0,0 +1,159 @@ +From: Sebastian Kügler <se...@kde.org> +Date: Wed, 01 Jun 2016 14:54:16 +0000 +Subject: address race condition around setoperation +X-Git-Tag: v5.6.95 +X-Git-Url: http://quickgit.kde.org/?p=kscreen.git&a=commitdiff&h=17199d32f292f7c44eb8cdce5b35396d3bd19eb8 +--- +address race condition around setoperation + +Summary: +Use a timer to avoid catching configChanged signals after we set +changes. + +The long version: + +TL;DR: We have a race condition when the kscreen daemon starts. It looks +up a known config, then applies it and subsequently resaves the config. +The same happens on config changes, it writes, then re-reads and then +re-writes the config change. +I've managed to prevent this from happening by adding a timer that does +avoids saving the config as a direct reaction to our own config changes. + +So what happens on kded5 startup after loading the kscreen2 module: + +- the kscreen config is requested and received +- the kscreen daemon (KD) looks into its config directory for a suitable +config file +(a config file is identified by a combined hash of all screen +attached, so unique per connected set of outputs) +- KD usually finds a config +- KD ignores configChanged events before it starts ... +- a KScreen::SetConfigOperation to apply the "known config" +- SetConfigOperation returns after a while (say 100ms later) +- we re-enable the change monitor +- we receive a configChanged signal +- we save the new config (usually to the existing config file) + +I don't think this behavior is desirable. I don't see a reason why the +daemon should save its config right after applying it. I think this +causes more problems than we want, since the startup may overwrite the +user's config. This behavior seems to be desired by the code in KD, it's +already blocking configChanged signals during the SetOperation (which, +to be honest may result in nightmarish behavior in any way, so it might +be a kludge which aims too short). + +From libkscreen perspective, SetConfigOperation::finished cannot +guarantee that all configChanged signals are already fired and that it's +safe to watch for new, independent changes now. At least on X11, we +simply don't know, and what we can do is wait a bit and cross fingers +that we're not catching our own noise. The changed signal *may* come +from a re-request of the edid information, but this is a bit hard to +track down, and not too useful, anyway, since changed Edid may affect a +large number of a screen's properties. +In the Wayland backend, that's a different story and we can prevent this +behavior at an earlier stage, so this timer is "probably not needed" (I +haven't tested that). + +This effectively prevents KD from catching reactions to its own changes +and does not trigger saving the config file on every login. It still +reacts to changes from libkscreen, but will avoid re-saving the config a +few times. The timer may not be the neatest of solutions for this, but +it does help narrowing down the problem and may be a last resort action. +Most importantly, it avoids the re-writing of the config on startup and +plugging/unplugging a monitor effectively. + +The timer value of 100ms is also used in kwin, which should make the +behavior (which is no problem in kwin) more solid. + +CCBUG:346961 +CCBUG:358011 + +Reviewers: graesslin + +Reviewed By: graesslin + +Subscribers: plasma-devel, #plasma + +Tags: #plasma + +Differential Revision: https://phabricator.kde.org/D1730 +--- + + +--- a/kded/daemon.cpp ++++ b/kded/daemon.cpp +@@ -23,6 +23,7 @@ + #include "kscreenadaptor.h" + #include "debug.h" + ++#include <QElapsedTimer> + #include <QTimer> + #include <QAction> + #include <QShortcut> +@@ -52,6 +53,7 @@ + , m_buttonTimer(new QTimer()) + , m_saveTimer(new QTimer()) + , m_lidClosedTimer(new QTimer()) ++ , m_changeBlockTimer(new QElapsedTimer()) + + { + QMetaObject::invokeMethod(this, "requestConfig", Qt::QueuedConnection); +@@ -82,6 +84,7 @@ + delete m_saveTimer; + delete m_buttonTimer; + delete m_lidClosedTimer; ++ delete m_changeBlockTimer; + + Generator::destroy(); + Device::destroy(); +@@ -112,7 +115,6 @@ + m_lidClosedTimer->setInterval(1000); + m_lidClosedTimer->setSingleShot(true); + connect(m_lidClosedTimer, &QTimer::timeout, this, &KScreenDaemon::lidClosedTimeout); +- + + connect(Device::self(), &Device::lidClosedChanged, this, &KScreenDaemon::lidClosedChanged); + connect(Device::self(), &Device::resumingFromSuspend, +@@ -145,6 +147,9 @@ + connect(new KScreen::SetConfigOperation(config), &KScreen::SetConfigOperation::finished, + [&]() { + qCDebug(KSCREEN_KDED) << "Config applied"; ++ // We enable monitoring already, but we will ignore the first signals that come ++ // in the next 100ms, since these are likely our own changes still flushing out ++ m_changeBlockTimer->start(); + setMonitorForChanges(true); + }); + } +@@ -182,6 +187,12 @@ + + void KScreenDaemon::configChanged() + { ++ if (m_changeBlockTimer->isValid() && !m_changeBlockTimer->hasExpired(100)) { ++ m_changeBlockTimer->start(); ++ qCDebug(KSCREEN_KDED) << "Change detected, but ignoring since it's our own noise"; ++ return; ++ } ++ m_changeBlockTimer->invalidate(); + qCDebug(KSCREEN_KDED) << "Change detected"; + // Reset timer, delay the writeback + m_saveTimer->start(); + +--- a/kded/daemon.h ++++ b/kded/daemon.h +@@ -27,6 +27,7 @@ + + #include "generator.h" + ++class QElapsedTimer; + class QTimer; + + namespace KScreen +@@ -79,6 +80,7 @@ + QTimer* m_buttonTimer; + QTimer* m_saveTimer; + QTimer* m_lidClosedTimer; ++ QElapsedTimer* m_changeBlockTimer; + }; + + #endif /*KSCREN_DAEMON_H*/ + diff --git a/kde-plasma/kscreen/kscreen-5.6.5-r1.ebuild b/kde-plasma/kscreen/kscreen-5.6.5-r1.ebuild new file mode 100644 index 0000000..a219d75 --- /dev/null +++ b/kde-plasma/kscreen/kscreen-5.6.5-r1.ebuild @@ -0,0 +1,40 @@ +# Copyright 1999-2016 Gentoo Foundation +# Distributed under the terms of the GNU General Public License v2 +# $Id$ + +EAPI=6 + +KDE_TEST="forceoptional" +inherit kde5 + +DESCRIPTION="KDE Plasma screen management" +HOMEPAGE="https://projects.kde.org/projects/extragear/base/kscreen"; + +KEYWORDS="~amd64 ~arm ~x86" +IUSE="" + +DEPEND=" + $(add_frameworks_dep kconfig) + $(add_frameworks_dep kconfigwidgets) + $(add_frameworks_dep kcoreaddons) + $(add_frameworks_dep kdbusaddons) + $(add_frameworks_dep kglobalaccel) + $(add_frameworks_dep ki18n) + $(add_frameworks_dep kwidgetsaddons) + $(add_frameworks_dep kxmlgui) + $(add_plasma_dep libkscreen) + $(add_qt_dep qtdbus) + $(add_qt_dep qtdeclarative 'widgets') + $(add_qt_dep qtgui) + $(add_qt_dep qtwidgets) +" +RDEPEND="${DEPEND} + $(add_plasma_dep kde-cli-tools) + $(add_qt_dep qtgraphicaleffects) + !kde-misc/kscreen +" + +PATCHES=( "${FILESDIR}/${P}-config-fix.patch" ) + +# bug #580440, last checked 5.6.3 +RESTRICT="test"
