Differential D7481 Diff 30822 libtaskmanager/xwindowsystemeventbatcher.cpp

Changeset View

Standalone View

View Options

libtaskmanager/xwindowsystemeventbatcher.cpp

  • This file was added.
1​/********************************************************************
2​Copyright 2017 David Edmundson <davidedmundson@kde.org>
3​
4​This library is free software; you can redistribute it and/or
5​modify it under the terms of the GNU Lesser General Public
6​License as published by the Free Software Foundation; either
7​version 2.1 of the License, or (at your option) version 3, or any
8​later version accepted by the membership of KDE e.V. (or its
9​successor approved by the membership of KDE e.V.), which shall
10​act as a proxy defined in Section 6 of version 3 of the license.
11​
12​This library is distributed in the hope that it will be useful,
13​but WITHOUT ANY WARRANTY; without even the implied warranty of
14​MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15​Lesser General Public License for more details.
16​
17​You should have received a copy of the GNU Lesser General Public
18​License along with this library. If not, see <http://www.gnu.org/licenses/>.
19​*********************************************************************/
20​
21​#include "xwindowsystemeventbatcher.h"
22​
23​#include <KWindowSystem>
24​
25​#define BATCH_TIME 10
26​
27​static const NET::Properties s_cachableProperties = NET::WMName | NET::WMVisibleName;
28​static const NET::Properties2 s_cachableProperties2 = NET::WM2UserTime;
29​
30​XWindowSystemEventBatcher::XWindowSystemEventBatcher(QObject* parent)
31​ : QObject(parent)
32​{
33​ connect(KWindowSystem::self(), &KWindowSystem::windowAdded, this, &XWindowSystemEventBatcher::windowAdded);
34​
35​ //remove our cache entries when we lose a window, otherwise we might fire change signals after a window is destroyed which wouldn't make sense
broulikUnsubmitted
Not Done

Avoid double lookup

broulik: Avoid double lookup
36​ connect(KWindowSystem::self(), &KWindowSystem::windowRemoved, this, [this](WId wid) {
broulikUnsubmitted
Not Done

Why emit a window change just before you emit a removal? Or is that what KWindowSystem usually does and we rely on that?

broulik: Why emit a window change just before you emit a removal? Or is that what `KWindowSystem`…
davidedmundsonAuthorUnsubmitted
Not Done

In a version where we cache all properties, I didn't want to have anything important missed.

If we only cache the name, then you're right, I don't need to.

davidedmundson: In a version where we cache all properties, I didn't want to have anything important missed.
37​ m_cache.remove(wid);
38​ emit windowRemoved(wid);
39​ });
40​
41​ void (KWindowSystem::*myWindowChangeSignal)(WId window,
42​ NET::Properties properties, NET::Properties2 properties2) = &KWindowSystem::windowChanged;
43​ QObject::connect(KWindowSystem::self(), myWindowChangeSignal, this,
44​ [this](WId window, NET::Properties properties, NET::Properties2 properties2) {
45​ //if properties contained only cachable flags
46​ if ((properties | s_cachableProperties) == s_cachableProperties &&
broulikUnsubmitted
Not Done

Look up only once by caching reference?

broulik: Look up only once by caching reference?
47​ (properties2 | s_cachableProperties2) == s_cachableProperties2) {
48​ m_cache[window].properties |= properties;
49​ m_cache[window].properties2 |= properties2;
graesslinUnsubmitted
Not Done

if we can depend on Qt 5.9 you could simplify the BATCH_TIME to either:

startTimer(std::chrono::milliseconds(10));

or together with C++14 to:

using namespace std::chrono_literals;
startTimer(10ms);
graesslin: if we can depend on Qt 5.9 you could simplify the BATCH_TIME to either: ``` startTimer(std…
50​ if (!m_timerId) {
51​ m_timerId = startTimer(BATCH_TIME);
52​ }
53​ } else {
54​ //submit all caches along with any real updates
55​ auto it = m_cache.constFind(window);
56​ if (it != m_cache.constEnd()) {
57​ properties |= it->properties;
58​ properties2 |= it->properties2;
broulikUnsubmitted
Not Done

Coding style

broulik: Coding style
59​ m_cache.erase(it);
60​ }
61​ emit windowChanged(window, properties, properties2);
62​ }
63​ }
64​ );
65​}
66​
67​void XWindowSystemEventBatcher::timerEvent(QTimerEvent* event)
68​{
69​ Q_UNUSED(event);
broulikUnsubmitted
Not Done

Please check event->timerId() against m_timerId, plasmashell keeps hogging 100% CPU when I have certain windows focused (e.g. ksysguard) as this method is entered a ton of times with timer ids other than our own.

broulik: Please check `event->timerId()` against `m_timerId`, plasmashell keeps hogging 100% CPU when I…
broulikUnsubmitted
Not Done

Even with that plasmashell constantly consumes a ton of CPU and becomes sluggish depending on what window is currently focused and what you're doing :/ Perhaps QBasicTimer is a better choice?

broulik: Even with that plasmashell constantly consumes a ton of CPU and becomes sluggish depending on…
70​ for (auto it = m_cache.constBegin(); it!= m_cache.constEnd(); it++) {
71​ emit windowChanged(it.key(), it.value().properties, it.value().properties2);
72​ };
73​ m_cache.clear();
74​ m_timerId = 0;
75​}