From 630514d52ab7211f7857a49a907dcde94523495f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= <mgraesslin@kde.org>
Date: Wed, 21 Jun 2017 21:10:12 +0200
Subject: [PATCH] Remove roundtrip to XServer from Workspace::xStackingOrder
 Introduce a method Workspace::markXStackingOrderAsDirty

Summary:
This method replaces the calls x_stacking_dirty = true in the code base
allowing for further refactoring of that functionality.

Remove roundtrip to XServer from Workspace::xStackingOrder

The method xStackingOrder is only used during a Compositor paint pass.
If the stacking order had changed, the method updated the stacking order
from X by performing a sync XQueryTree. With other words we had a round
trip to the X server directly in the paint pass.

This change rearchitectures this area by making better use of xcb. When
we notice that the stacking order changed and an XQueryTree is needed,
we directly send out the request. When xStackingOrder is finally called,
which normally happens a few milliseconds later, the reply is retreived.
In the worst case it still blocks, but in most cases the roundtrip is
gone.

If the stacking order changed again before accessing xStackingOrder the
running request is cancelled and a new request is issued. So whenever we
get into xStackingOrder it will have the current state.

The updating of the xStackingOrder is moved into a dedicated method and
xStackingOrder invokes it through a const_cast instead of operating on
mutable variables.

Test Plan: Normal system usage, no issues

Reviewers: #kwin, #plasma

Subscribers: plasma-devel, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D6323
---
 composite.cpp |  1 +
 layers.cpp    | 20 ++++++++++++--------
 workspace.cpp |  3 +--
 workspace.h   | 10 +++++++---
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/composite.cpp b/composite.cpp
index c36d83164a..4c848f04d5 100644
--- a/composite.cpp
+++ b/composite.cpp
@@ -296,6 +296,7 @@ void Compositor::startupWithWorkspace()
     if (!m_starting) {
         return;
     }
+    Workspace::self()->markXStackingOrderAsDirty();
     Q_ASSERT(m_scene);
     connect(workspace(), &Workspace::destroyed, this, [this] { compositeTimer.stop(); });
     claimCompositorSelection();
diff --git a/layers.cpp b/layers.cpp
index 8290bf3730..1734d0cb6c 100644
--- a/layers.cpp
+++ b/layers.cpp
@@ -687,18 +687,23 @@ bool Workspace::keepTransientAbove(const AbstractClient* mainwindow, const Abstr
 // Returns all windows in their stacking order on the root window.
 ToplevelList Workspace::xStackingOrder() const
 {
-    if (!x_stacking_dirty)
-        return x_stacking;
-    x_stacking_dirty = false;
+    if (m_xStackingQueryTree) {
+        const_cast<Workspace*>(this)->updateXStackingOrder();
+    }
+    return x_stacking;
+}
+
+void Workspace::updateXStackingOrder()
+{
     x_stacking.clear();
-    Xcb::Tree tree(rootWindow());
+    std::unique_ptr<Xcb::Tree> tree{std::move(m_xStackingQueryTree)};
     // use our own stacking order, not the X one, as they may differ
     foreach (Toplevel * c, stacking_order)
     x_stacking.append(c);
 
-    if (!tree.isNull()) {
-        xcb_window_t *windows = tree.children();
-        const auto count = tree->children_len;
+    if (!tree->isNull()) {
+        xcb_window_t *windows = tree->children();
+        const auto count = tree->data()->children_len;
         int foundUnmanagedCount = unmanaged.count();
         for (unsigned int i = 0;
                 i < count;
@@ -724,7 +729,6 @@ ToplevelList Workspace::xStackingOrder() const
             }
         }
     }
-    return x_stacking;
 }
 
 //*******************************
diff --git a/workspace.cpp b/workspace.cpp
index c7c9d1dff7..5f60a6af9e 100644
--- a/workspace.cpp
+++ b/workspace.cpp
@@ -112,7 +112,6 @@ Workspace::Workspace(const QString &sessionKey)
     , movingClient(0)
     , delayfocus_client(0)
     , force_restacking(false)
-    , x_stacking_dirty(true)
     , showing_desktop(false)
     , was_user_interaction(false)
     , session_saving(false)
@@ -1768,7 +1767,7 @@ Toplevel *Workspace::findInternal(QWindow *w) const
 
 void Workspace::markXStackingOrderAsDirty()
 {
-    x_stacking_dirty = true;
+    m_xStackingQueryTree.reset(new Xcb::Tree(rootWindow()));
 }
 
 } // namespace
diff --git a/workspace.h b/workspace.h
index b4487dc51e..e6a983496d 100644
--- a/workspace.h
+++ b/workspace.h
@@ -32,6 +32,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <QVector>
 // std
 #include <functional>
+#include <memory>
 
 // TODO: Cleanup the order of things in this .h file
 
@@ -47,6 +48,7 @@ namespace KWin
 
 namespace Xcb
 {
+class Tree;
 class Window;
 }
 
@@ -365,6 +367,8 @@ public:
     void registerEventFilter(X11EventFilter *filter);
     void unregisterEventFilter(X11EventFilter *filter);
 
+    void markXStackingOrderAsDirty();
+
 public Q_SLOTS:
     void performWindowOperation(KWin::AbstractClient* c, Options::WindowOperation op);
     // Keybindings
@@ -546,7 +550,7 @@ private:
     static NET::WindowType txtToWindowType(const char* txt);
     static bool sessionInfoWindowTypeMatch(Client* c, SessionInfo* info);
 
-    void markXStackingOrderAsDirty();
+    void updateXStackingOrder();
 
     AbstractClient* active_client;
     AbstractClient* last_active_client;
@@ -567,8 +571,8 @@ private:
     ToplevelList unconstrained_stacking_order; // Topmost last
     ToplevelList stacking_order; // Topmost last
     bool force_restacking;
-    mutable ToplevelList x_stacking; // From XQueryTree()
-    mutable bool x_stacking_dirty;
+    ToplevelList x_stacking; // From XQueryTree()
+    std::unique_ptr<Xcb::Tree> m_xStackingQueryTree;
     QList<AbstractClient*> should_get_focus; // Last is most recent
     QList<AbstractClient*> attention_chain;