From 1ff2846579bfd937235aefc22dd308e040352ef6 Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Wed, 21 Aug 2024 12:32:51 +0000
Subject: [PATCH] Remove boolean trap in Workspace::stackBelow()

Boolean traps are a code smell that makes code harder to read and it
also usually makes the api more difficult.

The force == false is used by two places:

 - in Workspace::restackWindowUnderActive()
 - and in X11Window::restackWindow()

Technically, the restackWindow() function doesn't need the force == false
behavior because the stackBelow() function is used in code paths where an
explicit sibling is provided.

The restackWindowUnderActive() function is trickier, it is used in the case
when kwin rejects to raise a newly mapped window, so for that purpose, I
changed the function so it picks the lowest window that belongs to the same
application as m_activeWindow.

The result of this change is that the stackBelow() is simpler and its
behavior is much more predictable, which is important when analyzing the
code that updates the stack.
---
 src/layers.cpp        | 27 +++++++++++++++------------
 src/tabbox/tabbox.cpp |  2 +-
 src/workspace.h       |  2 +-
 src/x11window.cpp     |  2 +-
 4 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/src/layers.cpp b/src/layers.cpp
index a39a80835f..d5a6faf5c6 100644
--- a/src/layers.cpp
+++ b/src/layers.cpp
@@ -445,22 +445,12 @@ void Workspace::lowerWindowRequest(Window *window)
     lowerWindowWithinApplication(window);
 }
 
-void Workspace::stackBelow(Window *window, Window *reference, bool force)
+void Workspace::stackBelow(Window *window, Window *reference)
 {
     if (window->isDeleted()) {
         qCWarning(KWIN_CORE) << "Workspace::stackBelow: closed window" << window << "cannot be restacked";
         return;
     }
-    if (!force && !Window::belongToSameApplication(reference, window)) {
-        // put in the stacking order below _all_ windows belonging to the active application
-        for (int i = 0; i < unconstrained_stacking_order.size(); ++i) {
-            auto other = unconstrained_stacking_order.at(i);
-            if (other->isClient() && other->layer() == window->layer() && Window::belongToSameApplication(reference, other)) {
-                reference = other;
-                break;
-            }
-        }
-    }
 
     Q_ASSERT(unconstrained_stacking_order.contains(reference));
     if (reference == window) {
@@ -480,7 +470,20 @@ void Workspace::restackWindowUnderActive(Window *window)
         raiseWindow(window);
         return;
     }
-    stackBelow(window, m_activeWindow);
+
+    Window *reference = m_activeWindow;
+    if (!Window::belongToSameApplication(reference, window)) {
+        // put in the stacking order below _all_ windows belonging to the active application
+        for (int i = 0; i < unconstrained_stacking_order.size(); ++i) {
+            auto other = unconstrained_stacking_order.at(i);
+            if (other->isClient() && other->layer() == window->layer() && Window::belongToSameApplication(reference, other)) {
+                reference = other;
+                break;
+            }
+        }
+    }
+
+    stackBelow(window, reference);
 }
 
 #if KWIN_BUILD_X11
diff --git a/src/tabbox/tabbox.cpp b/src/tabbox/tabbox.cpp
index b5b1475041..24ce6602f6 100644
--- a/src/tabbox/tabbox.cpp
+++ b/src/tabbox/tabbox.cpp
@@ -226,7 +226,7 @@ void TabBoxHandlerImpl::raiseClient(Window *c) const
 
 void TabBoxHandlerImpl::restack(Window *c, Window *under)
 {
-    Workspace::self()->stackBelow(c, under, true);
+    Workspace::self()->stackBelow(c, under);
 }
 
 void TabBoxHandlerImpl::elevateClient(Window *c, QWindow *tabbox, bool b) const
diff --git a/src/workspace.h b/src/workspace.h
index e2a7ae2893..165001c09d 100644
--- a/src/workspace.h
+++ b/src/workspace.h
@@ -235,7 +235,7 @@ public:
 #endif
     void lowerWindowRequest(Window *window);
     void restackWindowUnderActive(Window *window);
-    void stackBelow(Window *window, Window *reference, bool force = false);
+    void stackBelow(Window *window, Window *reference);
     void raiseOrLowerWindow(Window *window);
     void updateStackingOrder(bool propagate_new_windows = false);
     void forceRestacking();
diff --git a/src/x11window.cpp b/src/x11window.cpp
index e92d6ad3c1..b4b32dc1ca 100644
--- a/src/x11window.cpp
+++ b/src/x11window.cpp
@@ -1738,7 +1738,7 @@ void X11Window::doSetShade(ShadeMode previousShadeMode)
         shade_geometry_change = false;
         if (previousShadeMode == ShadeHover) {
             if (shade_below && workspace()->stackingOrder().indexOf(shade_below) > -1) {
-                workspace()->stackBelow(this, shade_below, true);
+                workspace()->stackBelow(this, shade_below);
             }
             if (isActive()) {
                 workspace()->activateNextWindow(this);