[gentoo-commits] repo/gentoo:master commit in: kde-plasma/kscreen/, kde-plasma/kscreen/files/

[Michael Palimaka]

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 <sebas@×××.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"