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.
This commit is contained in:
Vlad Zahorodnii 2024-08-21 12:32:51 +00:00
parent b6c1f7fb51
commit 1ff2846579
4 changed files with 18 additions and 15 deletions

View file

@ -445,22 +445,12 @@ void Workspace::lowerWindowRequest(Window *window)
lowerWindowWithinApplication(window); lowerWindowWithinApplication(window);
} }
void Workspace::stackBelow(Window *window, Window *reference, bool force) void Workspace::stackBelow(Window *window, Window *reference)
{ {
if (window->isDeleted()) { if (window->isDeleted()) {
qCWarning(KWIN_CORE) << "Workspace::stackBelow: closed window" << window << "cannot be restacked"; qCWarning(KWIN_CORE) << "Workspace::stackBelow: closed window" << window << "cannot be restacked";
return; 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)); Q_ASSERT(unconstrained_stacking_order.contains(reference));
if (reference == window) { if (reference == window) {
@ -480,7 +470,20 @@ void Workspace::restackWindowUnderActive(Window *window)
raiseWindow(window); raiseWindow(window);
return; 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 #if KWIN_BUILD_X11

View file

@ -226,7 +226,7 @@ void TabBoxHandlerImpl::raiseClient(Window *c) const
void TabBoxHandlerImpl::restack(Window *c, Window *under) 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 void TabBoxHandlerImpl::elevateClient(Window *c, QWindow *tabbox, bool b) const

View file

@ -235,7 +235,7 @@ public:
#endif #endif
void lowerWindowRequest(Window *window); void lowerWindowRequest(Window *window);
void restackWindowUnderActive(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 raiseOrLowerWindow(Window *window);
void updateStackingOrder(bool propagate_new_windows = false); void updateStackingOrder(bool propagate_new_windows = false);
void forceRestacking(); void forceRestacking();

View file

@ -1738,7 +1738,7 @@ void X11Window::doSetShade(ShadeMode previousShadeMode)
shade_geometry_change = false; shade_geometry_change = false;
if (previousShadeMode == ShadeHover) { if (previousShadeMode == ShadeHover) {
if (shade_below && workspace()->stackingOrder().indexOf(shade_below) > -1) { if (shade_below && workspace()->stackingOrder().indexOf(shade_below) > -1) {
workspace()->stackBelow(this, shade_below, true); workspace()->stackBelow(this, shade_below);
} }
if (isActive()) { if (isActive()) {
workspace()->activateNextWindow(this); workspace()->activateNextWindow(this);